Re: [PATCH v2 2/4] pathspec: strip multiple trailing slashes from submodules

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 13, 2013 at 3:21 AM, John Keeping <john@xxxxxxxxxxxxx> wrote:
> On Thu, Sep 12, 2013 at 12:48:10PM -0700, Junio C Hamano wrote:
>> John Keeping <john@xxxxxxxxxxxxx> writes:
>>
>> > This allows us to replace the submodule path trailing slash removal in
>> > builtin/rm.c with the PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP flag to
>> > parse_pathspec() without changing the behaviour with respect to multiple
>> > trailing slashes.
>>
>> Where does prefix_pathspec()'s input, which could have an unwanted
>> trailing slash, come from?
>>
>> If it is read from some of our internal data structure and known to
>> have at most one, then this change makes me feel very uneasy to cope
>> with potentially sloppy end-user input and data generated by ourselves
>> with the same logic.  It will allow our internal to be sloppy without
>> forcing us notice and fix that sloppiness.
>>
>> If it is coming from an end-user input, then I would not object to
>> the change, though.
>
> I added this in response to Duy's comment on v1 [1].
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/234548

Looks like I add more noise to this thread than anything valuable.
Yes, once argv goes through parse_pathspec it's normalized so we do
not need to strip consecutive slashes any more. I'm not entirely sure
if it also converts Windows '\\' to '/'..

> Looking more closely, this does come from user input (via the argv
> passed into parse_pathspec) but does (some of the time) go through
> prefix_path_gently which calls normalize_path_copy_len.
>
> It's not immediately clear to me when prefix_pathspec goes through this
> particular code path, but I think we may be able to drop this (and the
> previous patch) without affecting the user.
-- 
Duy
--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]