On Wed, Aug 03, 2016 at 01:11:51PM -0700, Stefan Beller wrote:
> > Coverity complains about "sub" being NULL here, and indeed, it seems
> > like an easy segfault:
> >
> > $ ./git submodule--helper remote-branch foo
> > Segmentation fault
> >
> > I guess this should return NULL in that case. But then this...
>
> I thought about checking for (!sub && !sub->branch) to return "master";
> However I disregarded that implementation as the submodule-config is
> reliable for any correct input. Though we need to handle wrong input as
> well. So how about:
>
> if (!sub)
> die(_("A submodule doesn't exist at path %s"), path);
> if (!sub->branch)
> return "master";
OK by me, as long as the caller of submodule--helper handles the death
reasonably. Since it's an internal helper program, we really only have
to worry about what git-submodule.sh does with it, which is good.
> > would need to handle the NULL return, as well. So maybe "master" or the
> > empty string would be better. I haven't followed the topic closely
> > enough to have an opinion.
>
> Oh I see, this can turn into a discussion what the API for
> remote_submodule_branch is. I think handling the NULL here and dying makes
> more sense as then we can keep remote_submodule_branch pure to its
> intended use case. (If in the far future we have all the submodule stuff in C,
> we may want to call remote_submodule_branch when we already know
> that the path is valid, so no need to check it in there. Also not dying in there
> is more libified.)
Yep, that makes sense to me, too.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html