diff options
author | Emily Shaffer <emilyshaffer@google.com> | 2019-05-23 13:23:56 -0700 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2019-05-28 10:57:07 -0700 |
commit | de99eb0c24d96730de187edd37b9ca9d42b0b05d (patch) | |
tree | d850dfb2e04809517b048db9733dea477823b3f6 | |
parent | mingw: allow building with an MSYS2 runtime v3.x (diff) | |
download | tgif-de99eb0c24d96730de187edd37b9ca9d42b0b05d.tar.xz |
grep: fail if call could output and name is null
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
grep_source.name be non-NULL
Aborted
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 <emilyshaffer@google.com>
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
-rw-r--r-- | grep.c | 4 |
1 files changed, 4 insertions, 0 deletions
@@ -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 " + "grep_source.name be non-NULL"); + if (!opt->output) opt->output = std_output; |