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