On Fri, Sep 13, 2013 at 08:28:24AM +0700, Duy Nguyen wrote: > 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 '/'.. If it goes through normalize_path_copy_len then it does. Junio, can you please drop the first two patches in this series? I can resend the final two unmodified if necessary, but I suspect it's easier for you to just drop the commits. > > 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. -- 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