Re: Bug: problem with file named with dash character

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

 



On Wed, Jun 27, 2012 at 03:52:05PM -0400, Jeff King wrote:

> I think you'd want to do just do something like:
> 
> diff --git a/diff.c b/diff.c
> index 1a594df..aac72b7 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -2684,9 +2684,6 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
>  		struct stat st;
>  		int fd;
>  
> -		if (!strcmp(s->path, "-"))
> -			return populate_from_stdin(s);
> -
>  		if (lstat(s->path, &st) < 0) {
>  			if (errno == ENOENT) {
>  			err_empty:
> 
> to temporarily fix it. That breaks
> 
>   echo content | git diff --no-index - some-file
> 
> but that code path should be fixed properly (with a use_stdin flag in
> the filespec).

Something like the patch below, which keeps the stdin test in t4002
working for me.

I suspect there are other problems lurking with the stdin case. For
example, we try to drop filespec contents whenever we can to reduce
memory pressure, under the assumption that we can always re-read the
blob later. But with stdin, we would need to be careful to mark the
contents as precious somehow.

diff --git a/diff-no-index.c b/diff-no-index.c
index f0b0010..c64bd5c 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -51,6 +51,21 @@ static int get_mode(const char *path, int *mode)
 	return 0;
 }
 
+static struct diff_filespec *noindex_filespec(const char *name, int mode)
+{
+	struct diff_filespec *r;
+
+	if (!name)
+		name = "/dev/null";
+	r = alloc_filespec(name);
+
+	fill_filespec(r, null_sha1, mode);
+	if (!strcmp(name, "-"))
+		r->is_stdin = 1;
+
+	return r;
+}
+
 static int queue_diff(struct diff_options *o,
 		      const char *name1, const char *name2)
 {
@@ -137,15 +152,8 @@ static int queue_diff(struct diff_options *o,
 			tmp_c = name1; name1 = name2; name2 = tmp_c;
 		}
 
-		if (!name1)
-			name1 = "/dev/null";
-		if (!name2)
-			name2 = "/dev/null";
-		d1 = alloc_filespec(name1);
-		d2 = alloc_filespec(name2);
-		fill_filespec(d1, null_sha1, mode1);
-		fill_filespec(d2, null_sha1, mode2);
-
+		d1 = noindex_filespec(name1, mode1);
+		d2 = noindex_filespec(name2, mode2);
 		diff_queue(&diff_queued_diff, d1, d2);
 		return 0;
 	}
diff --git a/diff.c b/diff.c
index 1a594df..449bfba 100644
--- a/diff.c
+++ b/diff.c
@@ -2675,6 +2675,9 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
 	if (size_only && 0 < s->size)
 		return 0;
 
+	if (s->is_stdin)
+		return populate_from_stdin(s);
+
 	if (S_ISGITLINK(s->mode))
 		return diff_populate_gitlink(s, size_only);
 
@@ -2684,9 +2687,6 @@ int diff_populate_filespec(struct diff_filespec *s, int size_only)
 		struct stat st;
 		int fd;
 
-		if (!strcmp(s->path, "-"))
-			return populate_from_stdin(s);
-
 		if (lstat(s->path, &st) < 0) {
 			if (errno == ENOENT) {
 			err_empty:
diff --git a/diffcore.h b/diffcore.h
index 8f32b82..be0739c 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -43,6 +43,7 @@ struct diff_filespec {
 	unsigned should_free : 1; /* data should be free()'ed */
 	unsigned should_munmap : 1; /* data should be munmap()'ed */
 	unsigned dirty_submodule : 2;  /* For submodules: its work tree is dirty */
+	unsigned is_stdin : 1;
 #define DIRTY_SUBMODULE_UNTRACKED 1
 #define DIRTY_SUBMODULE_MODIFIED  2
 	unsigned has_more_entries : 1; /* only appear in combined diff */
--
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]