Re: [PATCH] grep: remove "repo" arg from non-supporting funcs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 27, 2018 at 3:58 PM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
> As part of commit f9ee2fcdfa ("grep: recurse in-process using 'struct
> repository'", 2017-08-02), many functions in builtin/grep.c were
> converted to also take "struct repository *" arguments. Among them were
> grep_object() and grep_objects().
>
> However, at least grep_objects() was converted incompletely - it calls
> gitmodules_config_oid(), which references the_repository.
>
> But it turns out that the conversion was extraneous anyway - there has
> been no user-visible effect - because grep_objects() is never invoked
> except with the_repository.
>
> Revert the changes to grep_objects() and grep_object() (which conversion
> is also extraneous) to show that both these functions do not support
> repositories other than the_repository.

I'd rather convert gitmodules_config_oid instead of reverting the other
functions into a world without an arbitrary repository object.

>
> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> ---
> Patch 1/5 of your series is obviously correct.
>
> I investigated the change to grep_objects() in patch 2/5, and here is a
> patch summarizing my findings. Consider including this patch before 2/5
> (or before 1/5). You'll probably need to write
> "submodule_free(the_repository);" instead of what you have currently,
> but other than that, I don't think this affects your patch set much.

Thanks for looking at the patches!
I'd think this patch is orthogonal to the series, as this is about the effort
of converting parts of git-grep whereas this series is fixing a bug (by
converting parts of the submodule config machinery))?

Thanks,
Stefan



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux