Re: [PATCH v3 05/12] refs: move submodule slash stripping code to get_submodule_ref_store

[Duy Nguyen <pclouds@gmail.com>]

On Thu, Apr 20, 2017 at 5:02 AM, Johannes Sixt <j6t@kdbg.org> 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.

I've looked at the callers. Yes it is a path and is_dir_sep() should
be used. Since this has been like this in the current code, unless
there's some more changes in this series or you insist, I would hold
this change back, wait for this series to settle and submit it later
(I'll have to do that anyway to kill the get_main_store() call in this
function).
-- 
Duy