Re: [PATCH 1/2] fix passing a name for config from submodules

> > In commit 959b5455 we implemented the initial version of the submodule
> 
> Usually we refer to the commit by a triple of "abbrev. sha1 (date, subject).
> See d201a1ecd (2015-05-21, test_bitmap_walk: free bitmap with bitmap_free)
> for an example. Or ce41720ca (2015-04-02, blame, log: format usage strings
> similarly to those in documentation).
> 
> Apparently we put the subject first and then the date. I always did it
> the other way
> round, to there is no strict coding guide line, though it helps a lot to have an
> understanding for a) how long are we in the "broken" state already as well as
> b) what was the rationale for introducing it.

> > @@ -397,8 +397,10 @@ static const struct submodule *config_from(struct submodule_cache *cache,
> >                 return entry->config;
> >         }
> >
> > -       if (!gitmodule_sha1_from_commit(commit_sha1, sha1))
> > +       if (!gitmodule_sha1_from_commit(commit_sha1, sha1, &rev)) {
> > +               strbuf_release(&rev);
> >                 return NULL;
> 
> This is a reoccuring pattern below. Maybe it might make sense to
> just do a s/return.../ goto out/ and at that label we cleanup `rev` and `config`
> and return a result value?
> There are currently 6 early returns (not counting the 3 from the last switch),
> 4 of them return NULL, so that would result in just a "goto out", whereas 2
> return an actual value, they would need to assign the result value first before
> jumping out of the logic. I dunno, just food for though.

> > @@ -425,8 +432,9 @@ static const struct submodule *config_from(struct submodule_cache *cache,
> >         parameter.commit_sha1 = commit_sha1;
> >         parameter.gitmodules_sha1 = sha1;
> >         parameter.overwrite = 0;
> > -       git_config_from_mem(parse_config, "submodule-blob", "",
> > +       git_config_from_mem(parse_config, "submodule-blob", rev.buf,
> >                         config, config_size, &parameter);
> 
> Ok, this is the actual fix. Do you want to demonstrate its impact by adding
> one or two tests that failed before and now work?
> (As I was using the submodule config API most of the time with null_sha1
> to indicate we'd be looking at the current .gitmodules file in the worktree,
> the actual bug may have not manifested in the users of this API.
> But still, it would be nice to see what was broken?)