Re: [RFC PATCH] grep: provide sane default to grep_source struct

[Junio C Hamano <gitster@pobox.com>]

Emily Shaffer <emilyshaffer@google.com> writes:

> 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

That's an interesting problem.

Currently the only use of the function is to see if the log message
matches with the given pattern (yes/no), but it is conceivable that
new callers may want to prepare in-core data and use it to see if
that matches the pattern, or even to _show_ the lines that match the
pattern (possibly with its own .output callback).  Such a caller may
even make repeated calls to the function, and want to give hint in
the output to help users identify which in-core data produced hits,
which implies that a hardcoded "(in core)" is not such a great idea.
In a future where we support such a caller, the function signature
of grep_buffer() would change to include the name, and you would be
passing that to grep_source_init().

Until that happens, it would be OK to use a hardcoded "in-core"
constant here, but to futureproof for such a future, I think it
makes sense to have a check for a BUG() suggested by jrn---that
would become useful once callers of grep_buffer() can give names
prevent them from passing NULL when it later would be used.

> 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;