mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Emily Shaffer <>
Cc: Emily Shaffer <>,
	Jeff King <>, Jonathan Nieder <>
Subject: [PATCH v4] grep: fail if call could output and name is null
Date: Thu, 23 May 2019 13:23:56 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

grep_source(), which performs much of the work for Git's grep library,
allows passing an arbitrary struct grep_source which represents the text
which grep_source() should search to match a pattern in the provided
struct grep_opt. In most callers, the grep_source::name field is set to
an appropriate prefix to print before a colon when a result matches:

	README:Git is an Open Source project covered by the GNU General

One caller, grep_buffer(), leaves the grep_source::name field set to
NULL because there isn't enough context to determine an appropriate name
for this kind of output line. In practice, this has been fine: the only
caller of grep_buffer() is "git log --grep", and that caller sets
grep_opt::status_only, which disables output and only checks whether a
match exists. But this is brittle: a future caller can call
grep_buffer() without grep_opt::status_only set, and as soon as it hits
a match, grep_source() will try to print the match and segfault:

	(null):Git is an Open Source project covered by the GNU General

For example, a future caller might want to print all matching lines from
commits which match a regex.

Futureproof by diagnosing early a use of the API that could trigger that
condition, before we know whether the pattern matches:

	BUG: grep.c:1783: grep call which could print a name requires be non-NULL

This way, the caller's author gets an indication of how to fix the issue
- by providing grep_source::name or setting grep_opt::status_only - and
they are warned of the potential for a segfault unconditionally, rather
than only if there is a match.

Noticed while adding such a call to a tutorial on revision walks.

Signed-off-by: Emily Shaffer <>
Reviewed-by: Jonathan Nieder <>
Since v3, only the commit message has changed. Reworked based on
Jonathan Nieder's suggestions (with some modifications for readability).

 - Emily

 grep.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/grep.c b/grep.c
index 0d50598acd..f7c3a5803e 100644
--- a/grep.c
+++ b/grep.c
@@ -1780,6 +1780,10 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle
 	enum grep_context ctx = GREP_CONTEXT_HEAD;
 	xdemitconf_t xecfg;
+	if (!opt->status_only && gs->name == NULL)
+		BUG("grep call which could print a name requires "
+		    " be non-NULL");
 	if (!opt->output)
 		opt->output = std_output;

  parent reply	other threads:[~2019-05-23 20:24 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-16  2:00 [RFC PATCH] grep: provide sane default to grep_source struct Emily Shaffer
2019-05-16  3:07 ` Junio C Hamano
2019-05-16  3:13   ` Jonathan Nieder
2019-05-16  3:11 ` Jonathan Nieder
2019-05-16 21:05   ` Emily Shaffer
2019-05-16 21:44 ` [PATCH v2] " Emily Shaffer
2019-05-16 22:02   ` Jeff King
2019-05-21 23:52     ` Emily Shaffer
2019-05-22  0:17       ` Jonathan Nieder
2019-05-22  0:34   ` [PATCH v3] " Emily Shaffer
2019-05-22  0:58     ` Jonathan Nieder
2019-05-22  4:01     ` Jeff King
2019-05-23 20:23     ` Emily Shaffer [this message]
2019-05-23 20:34       ` [PATCH v4] grep: fail if call could output and name is null Jonathan Nieder
2019-05-28 17:57         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).