On Tue, Feb 14, 2017 at 6:55 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote: >> >> +/* >> + * Return the ref_store instance for the specified submodule. For the >> + * main repository, use submodule==NULL; such a call cannot fail. > > So now we have both a get_main as well as a get_submodule function, > but the submodule function can return the main as well? > > I'd rather see this as a BUG; or asking another way: > What is the difference between get_submodule_ref_store(NULL) > and get_main_ref_store() ? Technical debts :) In order to do that, we need to make sure _submodule() never takes NULL as main repo. But current code does. On example is revision.c where submodule==NULL is the main repo. In the end, I agree that get_submodule_ref_store(NULL) should be a bug. > As you went through all call sites (by renaming the function), we'd > be able to tell that there is no caller with NULL, or is it? Direct call sites are just middle men though, e.g. for_each_ref_submodule(). I'll attempt to clean this up at some point in future (probably at the same time I attempt to kill *_submodule ref api). But I think for now I'll just put a TODO or FIXME comment here. -- Duy