Re: [PATCH 3/7] grep: typesafe versions of grep_source_init

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Aug 11, 2021, 23:07 UTC
Message-ID: <YRRYOmwW9yKhvldn@satellite>
In-Reply-To: <YRREPNA8GPyKdWBr@google.com>

On Wed, Aug 11, 2021 at 02:42:20PM -0700, Emily Shaffer wrote:

Show 11 quoted lines

> On Tue, Aug 10, 2021 at 11:28:41AM -0700, Jonathan Tan wrote:
> > 
> > grep_source_init() can create "struct grep_source" objects and,
> > depending on the value of the type passed, some void-pointer parameters have
> > different meanings. Because one of these types (GREP_SOURCE_OID) will
> > require an additional parameter in a subsequent patch, take the
> > opportunity to increase clarity and type safety by replacing this
> > function with individual functions for each type.
> > 
> > Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> Like Junio said, it is very neat.

[Sorry for piggy-backing, I have already deleted the original mail :( ]

Just a quick note: grep_source_init_buf() is only called from grep.c:1833, before its definition at grep.c:1869, so it could be marked as static (as things stand). Do you anticipate any future callers from outside of grep.c? (after removing the declaration from grep.h, you would need to add a forward declaration or, better, move the definition to before the call (or the call (and grep_buffer()) after the definition)).

ATB, Ramsay Jones