From mboxrd@z Thu Jan 1 00:00:00 1970 From: Junio C Hamano Subject: Re: [PATCH v2 2/4] pathspec: strip multiple trailing slashes from submodules Date: Thu, 12 Sep 2013 12:48:10 -0700 Message-ID: References: <7c478c19da6ee3322ca87e77a90358a30178c286.1379013786.git.john@keeping.me.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: git@vger.kernel.org, Duy Nguyen , Jens Lehmann , Johannes Sixt To: John Keeping X-From: git-owner@vger.kernel.org Thu Sep 12 21:48:28 2013 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1VKCsE-0000NO-9I for gcvg-git-2@plane.gmane.org; Thu, 12 Sep 2013 21:48:18 +0200 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756469Ab3ILTsO (ORCPT ); Thu, 12 Sep 2013 15:48:14 -0400 Received: from b-pb-sasl-quonix.pobox.com ([208.72.237.35]:56726 "EHLO smtp.pobox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753526Ab3ILTsN (ORCPT ); Thu, 12 Sep 2013 15:48:13 -0400 Received: from smtp.pobox.com (unknown [127.0.0.1]) by b-sasl-quonix.pobox.com (Postfix) with ESMTP id F304D40418; Thu, 12 Sep 2013 19:48:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; s=sasl; bh=AyIs933LFv4DiL4NGKhwKzk2H44=; b=pCAtjr vIh6JCRSzWQ0KO1jMIyraX81VPXCrdzfqa45j9sdjl8gsxAzR7ZXQ+0h0OMULs/a SEKxKWNCIDPHwm9Su/Y/rIRiFirZtVIbK/15+PRp1X3k05E6x+GIIoJ1DvPacc8N 0j7lQNrOotUUr7V5DNnHMaP2IbhhzMWj63TlI= DomainKey-Signature: a=rsa-sha1; c=nofws; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; q=dns; s=sasl; b=V6eM1vYuhboyXbZDRuBwtGlVN6rlBnfQ h8uZri3qYxM/vse9IL6z7l+u3cqVb+9AyUvxcWolOUNpMjM+kv1P01w1VHI4OFGu qTyOQVszkxxY30wGsPl1EPjhRIvJlDsS+B64MSnSxK3VMw50cm7TfzbNvsl9xNyG N3uL3vcLrs0= Received: from b-pb-sasl-quonix.pobox.com (unknown [127.0.0.1]) by b-sasl-quonix.pobox.com (Postfix) with ESMTP id E82C540417; Thu, 12 Sep 2013 19:48:12 +0000 (UTC) Received: from pobox.com (unknown [72.14.226.9]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by b-sasl-quonix.pobox.com (Postfix) with ESMTPSA id 3F9AB4040F; Thu, 12 Sep 2013 19:48:12 +0000 (UTC) In-Reply-To: <7c478c19da6ee3322ca87e77a90358a30178c286.1379013786.git.john@keeping.me.uk> (John Keeping's message of "Thu, 12 Sep 2013 20:24:59 +0100") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.3 (gnu/linux) X-Pobox-Relay-ID: 41DC9B2A-1BE4-11E3-97ED-CA9B8506CD1E-77302942!b-pb-sasl-quonix.pobox.com Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: John Keeping 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. Thanks. > Signed-off-by: John Keeping > --- > pathspec.c | 27 +++++++++++++++++---------- > 1 file changed, 17 insertions(+), 10 deletions(-) > > diff --git a/pathspec.c b/pathspec.c > index 7c6963b..11b031a 100644 > --- a/pathspec.c > +++ b/pathspec.c > @@ -251,12 +251,16 @@ static unsigned prefix_pathspec(struct pathspec_item *item, > item->len = strlen(item->match); > item->prefix = prefixlen; > > - if ((flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) && > - (item->len >= 1 && is_dir_sep(item->match[item->len - 1])) && > - (i = cache_name_pos(item->match, item->len - 1)) >= 0 && > - S_ISGITLINK(active_cache[i]->ce_mode)) { > - item->len--; > - match[item->len] = '\0'; > + if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP) { > + size_t pathlen = item->len; > + while (pathlen > 0 && is_dir_sep(item->match[pathlen - 1])) > + pathlen--; > + > + if ((i = cache_name_pos(item->match, pathlen)) >= 0 && > + S_ISGITLINK(active_cache[i]->ce_mode)) { > + item->len = pathlen; > + match[item->len] = '\0'; > + } > } > > if (flags & PATHSPEC_STRIP_SUBMODULE_SLASH_EXPENSIVE) > @@ -271,11 +275,14 @@ static unsigned prefix_pathspec(struct pathspec_item *item, > !is_dir_sep(match[ce_len]) || > memcmp(ce->name, match, ce_len)) > continue; > - if (item->len == ce_len + 1) { > - /* strip trailing slash */ > + > + while (item->len > 0 && is_dir_sep(match[item->len - 1])) > item->len--; > - match[item->len] = '\0'; > - } else > + > + /* strip trailing slash */ > + match[item->len] = '\0'; > + > + if (item->len != ce_len) > die (_("Pathspec '%s' is in submodule '%.*s'"), > elt, ce_len, ce->name); > }