All of lore.kernel.org
 help / color / mirror / Atom feed
From: Emily Shaffer <emilyshaffer@google.com>
To: git@vger.kernel.org
Cc: jrn@google.com, Emily Shaffer <emilyshaffer@google.com>
Subject: [RFC PATCH] grep: provide sane default to grep_source struct
Date: Wed, 15 May 2019 19:00:23 -0700	[thread overview]
Message-ID: <20190516020023.61161-1-emilyshaffer@google.com> (raw)

grep_buffer creates a struct grep_source gs and calls grep_source()
with it. However, gs.name is null, which eventually produces a
segmentation fault in
grep_source()->grep_source_1()->show_line() when grep_opt.status_only is
not set.

This seems to be unreachable from existing commands but is reachable in
the event that someone rolls their own revision walk and neglects to set
rev_info->grep_filter->status_only. Conceivably, someone might want to
print all changed lines of all commits which match some regex.

Signed-off-by: Emily Shaffer <emilyshaffer@google.com>
---
Hiya,

I ran into this issue while I was working on a tutorial on rolling your
own revision walk. I didn't want to print all the lines associated with
a matching change, but it didn't seem good that a seemingly-sane
grep_filter config was segfaulting.

I don't know if adding a placeholder name is the right answer (hence RFC
patch).

Jonathan Nieder proposed alternatively adding some check to grep_source()
to ensure that if opt->status_only is unset, gs->name must be non-NULL
(and yell about it if not), as well as some extra comments indicating
what assumptions are made about the data coming into functions like
grep_source(). I'm fine with that as well (although I'm not sure it
makes sense semantically to require a name which the user probably can't
easily set, or else ban the user from printing LOC during grep). Mostly
I'm happy with any solution besides a segfault with no error logging :)

Thanks in advance for everyone's thoughts.
Emily


 grep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/grep.c b/grep.c
index 0d50598acd..fd84454faf 100644
--- a/grep.c
+++ b/grep.c
@@ -2045,7 +2045,7 @@ int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size)
 	struct grep_source gs;
 	int r;
 
-	grep_source_init(&gs, GREP_SOURCE_BUF, NULL, NULL, NULL);
+	grep_source_init(&gs, GREP_SOURCE_BUF, _("(in memory)"), NULL, NULL);
 	gs.buf = buf;
 	gs.size = size;
 
-- 
2.21.0.1020.gf2820cf01a-goog


             reply	other threads:[~2019-05-16  2:00 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-16  2:00 Emily Shaffer [this message]
2019-05-16  3:07 ` [RFC PATCH] grep: provide sane default to grep_source struct 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     ` [PATCH v4] grep: fail if call could output and name is null Emily Shaffer
2019-05-23 20:34       ` 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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=20190516020023.61161-1-emilyshaffer@google.com \
    --to=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=jrn@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* 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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.