On Tue, Feb 14, 2017 at 2:04 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: > 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. ok, fine with me. My line of thinking when originally responding was to put the FIXME comment at the caller site, and there we'd differentiate between the two cases and call the appropriate function. > -- > Duy