On Wed, Aug 11, 2021 at 02:42:20PM -0700, Emily Shaffer wrote: > 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@xxxxxxxxxx> > 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
