threads / patch / 12996

patchRe: [PATCH] git-fetch: Don't trigger a bus error when given the refspec "tag"

Subject: Re: [PATCH] git-fetch: Don't trigger a bus error when given the refspec "tag"

## tl;dr

3 messages between Apr 5, 2008 and Apr 5, 2008. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Kevin Ballard· Apr 5, 2008, 18:28 UTC · lore

[PATCH] git-fetch: Don't trigger a bus error when given the refspec "tag"

When git-fetch encounters the refspec "tag" it assumes that the next argument will be a tag name. If there is no next argument, it should die gracefully instead of erroring.

Signed-off-by: Kevin Ballard <kevin@sb.org>
---
 builtin-fetch.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Show changes to builtin-fetch.c +2 −0
diff --git a/builtin-fetch.c b/builtin-fetch.c
index a11548c..5841b3e 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -637,6 +637,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 			if (!strcmp(argv[i], "tag")) {
 				char *ref;
 				i++;
+				if (i >= argc)
+					die("You need to specify a tag name.");
 				ref = xmalloc(strlen(argv[i]) * 2 + 22);
 				strcpy(ref, "refs/tags/");
 				strcat(ref, argv[i]);

-- 
1.5.5.rc3.dirty
Johannes Schindelin· Apr 5, 2008, 18:33 UTC · re: Kevin Ballard · lore
Hi,
On Sat, 5 Apr 2008, Kevin Ballard wrote:
Show 10 quoted lines
> diff --git a/builtin-fetch.c b/builtin-fetch.c
> index a11548c..5841b3e 100644
> --- a/builtin-fetch.c
> +++ b/builtin-fetch.c
> @@ -637,6 +637,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>  			if (!strcmp(argv[i], "tag")) {
>  				char *ref;
>  				i++;
> +				if (i >= argc)
> +					die("You need to specify a tag name.");
Would it not be better to check in the if() above?  IOW
			if (i + 1 < argc && !strcmp(argv[i], "tag")) {
Hmm?

Ciao, Dscho

Kevin Ballard· Apr 5, 2008, 18:46 UTC · re: Johannes Schindelin · lore
On Apr 5, 2008, at 2:33 PM, Johannes Schindelin wrote:
Show 21 quoted lines
> Hi,
>
> On Sat, 5 Apr 2008, Kevin Ballard wrote:
>
>> diff --git a/builtin-fetch.c b/builtin-fetch.c
>> index a11548c..5841b3e 100644
>> --- a/builtin-fetch.c
>> +++ b/builtin-fetch.c
>> @@ -637,6 +637,8 @@ int cmd_fetch(int argc, const char **argv,  
>> const char *prefix)
>> 			if (!strcmp(argv[i], "tag")) {
>> 				char *ref;
>> 				i++;
>> +				if (i >= argc)
>> +					die("You need to specify a tag name.");
>
> Would it not be better to check in the if() above?  IOW
>
> 			if (i + 1 < argc && !strcmp(argv[i], "tag")) {
>
> Hmm?

So that `git fetch origin tag` will try and fetch the branch named tag? Given that we treat "tag foo" as shorthand for refs/tags/foo:refs/ tags/foo it seems like it would be unexpected to treat "tag" as just a plain ref. I much prefer to die in this situation, let the user fix the refspec to mean exactly what they want (either by adding a tag name, or by using tag: if they really want a branch named tag).

-Kevin
-- 
Kevin Ballard
http://kevin.sb.org
kevin@sb.org
http://www.tildesoft.com

← back to recent threads