[PATCH] revision parsing: make "rev -- paths" checks stronger.

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

 



If you don't have a "--" marker, then:

 - all of the arguments we are going to assume are pathspecs
   must exist in the working tree.

 - none of the arguments we parsed as revisions could be
   interpreted as a filename.

so that there really isn't any possibility of confusion in case
somebody does have a revision that looks like a pathname too.

The former rule has been in effect; this implements the latter.

Signed-off-by: Junio C Hamano <junkio@xxxxxxx>
---

 * I did not understand 'if you have a pathspec, but' part, and
   'none of the paths are "--"' part of your rules, so this
   might probably be a bit different from what you had in
   mind.

   The patch, lacking the "if you have a pathspec" part of the
   rule, would make this complain:

   	$ >v1.0.0
        $ git show v1.0.0

   BTW, this would silently do what you would not find
   interesting, with or without the patch:

       $ >v2.9.6
	 ... long time later after you forget you had such a
	 ... bogus file
       $ git show v2.9.6

  Linus Torvalds <torvalds@xxxxxxxx> writes:

  > ... I was going to say that if you have a pathspec, 
  > but don't have a "--" marker, then we'd additionally check:
  >
  >  - none of the arguments we parsed as revisions could be interpreted as a 
  >    filename.
  >
  >  - none of the paths are "--" 
  >
  > so that there really isn't...



 cache.h    |    1 +
 revision.c |   14 +++++++++++++-
 setup.c    |   24 ++++++++++++++++++++++--
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index 4d8fabc..a4f253e 100644
--- a/cache.h
+++ b/cache.h
@@ -135,6 +135,7 @@ extern const char *setup_git_directory(v
 extern const char *prefix_path(const char *prefix, int len, const char *path);
 extern const char *prefix_filename(const char *prefix, int len, const char *path);
 extern void verify_filename(const char *prefix, const char *name);
+extern void verify_non_filename(const char *prefix, const char *name);
 
 #define alloc_nr(x) (((x)+16)*3/2)
 
diff --git a/revision.c b/revision.c
index f2a9f25..b6ed014 100644
--- a/revision.c
+++ b/revision.c
@@ -740,6 +740,11 @@ int setup_revisions(int argc, const char
 				include = get_reference(revs, next, sha1, flags);
 				if (!exclude || !include)
 					die("Invalid revision range %s..%s", arg, next);
+
+				if (!seen_dashdash) {
+					*dotdot = '.';
+					verify_non_filename(revs->prefix, arg);
+				}
 				add_pending_object(revs, exclude, this);
 				add_pending_object(revs, include, next);
 				continue;
@@ -757,13 +762,20 @@ int setup_revisions(int argc, const char
 			if (seen_dashdash || local_flags)
 				die("bad revision '%s'", arg);
 
-			/* If we didn't have a "--", all filenames must exist */
+			/* If we didn't have a "--":
+			 * (1) all filenames must exist;
+			 * (2) all rev-args must not be interpretable
+			 *     as a valid filename.
+			 * but the latter we have checked in the main loop.
+			 */
 			for (j = i; j < argc; j++)
 				verify_filename(revs->prefix, argv[j]);
 
 			revs->prune_data = get_pathspec(revs->prefix, argv + i);
 			break;
 		}
+		if (!seen_dashdash)
+			verify_non_filename(revs->prefix, arg);
 		object = get_reference(revs, arg, sha1, flags ^ local_flags);
 		add_pending_object(revs, object, arg);
 	}
diff --git a/setup.c b/setup.c
index cce9bb8..fe7f884 100644
--- a/setup.c
+++ b/setup.c
@@ -80,11 +80,31 @@ void verify_filename(const char *prefix,
 	if (!lstat(name, &st))
 		return;
 	if (errno == ENOENT)
-		die("ambiguous argument '%s': unknown revision or filename\n"
-		    "Use '--' to separate filenames from revisions", arg);
+		die("ambiguous argument '%s': unknown revision or path not in the working tree.\n"
+		    "Use '--' to separate paths from revisions", arg);
 	die("'%s': %s", arg, strerror(errno));
 }
 
+/*
+ * Opposite of the above: the command line did not have -- marker
+ * and we parsed the arg as a refname.  It should not be interpretable
+ * as a filename.
+ */
+void verify_non_filename(const char *prefix, const char *arg)
+{
+	const char *name;
+	struct stat st;
+
+	if (*arg == '-')
+		return; /* flag */
+	name = prefix ? prefix_filename(prefix, strlen(prefix), arg) : arg;
+	if (!lstat(name, &st))
+		die("ambiguous argument '%s': both revision and filename\n"
+		    "Use '--' to separate filenames from revisions", arg);
+	if (errno != ENOENT)
+		die("'%s': %s", arg, strerror(errno));
+}
+
 const char **get_pathspec(const char *prefix, const char **pathspec)
 {
 	const char *entry = *pathspec;
-- 
1.3.1.ga6c7


-
: 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]