On Thu, Apr 20, 2017 at 12:02:08AM +0200, Johannes Sixt wrote:
> Am 19.04.2017 um 13:01 schrieb Nguyễn Thái Ngọc Duy:
> > @@ -1558,7 +1543,17 @@ struct ref_store *get_submodule_ref_store(const char 
> > *submodule)
> >  {
> >     struct strbuf submodule_sb = STRBUF_INIT;
> >     struct ref_store *refs;
> > +   char *to_free = NULL;
> >     int ret;
> > +   size_t len;
> > +
> > +   if (submodule) {
> > +           len = strlen(submodule);
> > +           while (len && submodule[len - 1] == '/')
> 
> What is the source of the value of 'submodule'? Is it an index entry? Or 
> did it pass through parse_pathspec? In these cases it is correct to 
> compare against literal '/'. Otherwise, is_dir_sep() is preferred.

This is a code move from resolve_gitlink_ref(), which goes back to
0ebde32c87 (Add 'resolve_gitlink_ref()' helper function - 2007-04-09)
and it looks like a dir separator back then.

Can I convert that in a separate topic? I think Michael even wanted to
kill all these path manipulation in refs code, which makes sense, but
I would need to audit the callers carefully before making that move.
--
Duy

Reply via email to