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

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

[Kevin Ballard]

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(-)

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

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

[Johannes Schindelin]

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?

Ciao,
Dscho

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

[Kevin Ballard]

On Apr 5, 2008, at 2:33 PM, Johannes Schindelin wrote:
> 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