threads / patch / 5554

patch, 2 partsHandle invalid argc gently

Subject: [PATCH 2/2] Handle invalid argc gently

## tl;dr

7 messages between Sep 14, 2006 and Sep 14, 2006. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

Dmitry V. Levin· Sep 14, 2006, 01:04 UTC · lore

describe, git: Handle argc==0 case the same way as argc==1. merge-tree: Refuse excessive arguments.

Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
---
 describe.c   |    2 +-
 git.c        |    2 +-
 merge-tree.c |    2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
Show changes to 3 files +3 −3

describe.c, git.c, merge-tree.c

diff --git a/describe.c b/describe.c
index 5ed052d..2172e8f 100644
--- a/describe.c
+++ b/describe.c
@@ -163,7 +163,7 @@ int main(int argc, char **argv)
 
 	setup_git_directory();
 
-	if (i == argc)
+	if (i >= argc)
 		describe("HEAD", 1);
 	else
 		while (i < argc) {
diff --git a/git.c b/git.c
index 47c85e1..8c182a5 100644
--- a/git.c
+++ b/git.c
@@ -294,7 +294,7 @@ static void handle_internal_command(int 
 
 int main(int argc, const char **argv, char **envp)
 {
-	const char *cmd = argv[0];
+	const char *cmd = argv[0] ? argv[0] : "git-help";
 	char *slash = strrchr(cmd, '/');
 	const char *exec_path = NULL;
 	int done_alias = 0;
diff --git a/merge-tree.c b/merge-tree.c
index c154dcf..692ede0 100644
--- a/merge-tree.c
+++ b/merge-tree.c
@@ -337,7 +337,7 @@ int main(int argc, char **argv)
 	struct tree_desc t[3];
 	void *buf1, *buf2, *buf3;
 
-	if (argc < 4)
+	if (argc != 4)
 		usage(merge_tree_usage);
 
 	setup_git_directory();

-- 
ldv
Dmitry V. Levin· Sep 14, 2006, 01:31 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] Handle invalid argc gently

On Wed, Sep 13, 2006 at 06:12:02PM -0700, Junio C Hamano wrote:
Show 5 quoted lines
> "Dmitry V. Levin" <ldv@altlinux.org> writes:
> 
> > describe, git: Handle argc==0 case the same way as argc==1.
> 
> When does one get (ac == 0)?

When one executes something like execlp("git", NULL, NULL);

-- 
ldv
Junio C Hamano· Sep 14, 2006, 06:34 UTC · re: Dmitry V. Levin · lore

Re: [PATCH 2/2] Handle invalid argc gently

"Dmitry V. Levin" <ldv@altlinux.org> writes:
Show 9 quoted lines
> On Wed, Sep 13, 2006 at 06:12:02PM -0700, Junio C Hamano wrote:
>> "Dmitry V. Levin" <ldv@altlinux.org> writes:
>> 
>> > describe, git: Handle argc==0 case the same way as argc==1.
>> 
>> When does one get (ac == 0)?
>
> When one executes something like
> execlp("git", NULL, NULL);
Fair enough.
What is the valid reason to do execlp("git", NULL, NULL)?
Dmitry V. Levin· Sep 14, 2006, 13:16 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] Handle invalid argc gently

On Wed, Sep 13, 2006 at 11:34:05PM -0700, Junio C Hamano wrote:
Show 14 quoted lines
> "Dmitry V. Levin" <ldv@altlinux.org> writes:
> > On Wed, Sep 13, 2006 at 06:12:02PM -0700, Junio C Hamano wrote:
> >> "Dmitry V. Levin" <ldv@altlinux.org> writes:
> >> 
> >> > describe, git: Handle argc==0 case the same way as argc==1.
> >> 
> >> When does one get (ac == 0)?
> >
> > When one executes something like
> > execlp("git", NULL, NULL);
> 
> Fair enough.
> 
> What is the valid reason to do execlp("git", NULL, NULL)?
Personally I do not plan to execute git this way on regular basis, indeed. :)
But argc == 0 is allowed, so why should git crash?
-- 
ldv
Junio C Hamano· Sep 14, 2006, 16:53 UTC · re: Dmitry V. Levin · lore

Re: [PATCH 2/2] Handle invalid argc gently

"Dmitry V. Levin" <ldv@altlinux.org> writes:
Show 5 quoted lines
>> What is the valid reason to do execlp("git", NULL, NULL)?
>
> Personally I do not plan to execute git this way on regular basis, indeed. :)
>
> But argc == 0 is allowed, so why should git crash?

Oh, no I was not arguing for making git crash. I was just trying to learn if there is a valid reason to choose to, or common misconfiguration that causes it to, run with ac == 0, since I did not think of any.

It is good to be extra careful and defensive -- I just wanted to know if there is more to it.

Dmitry V. Levin· Sep 14, 2006, 18:25 UTC · re: Junio C Hamano · lore

Re: [PATCH 2/2] Handle invalid argc gently

On Thu, Sep 14, 2006 at 09:53:36AM -0700, Junio C Hamano wrote: [...]

Show 10 quoted lines
> >> What is the valid reason to do execlp("git", NULL, NULL)?
> >
> > Personally I do not plan to execute git this way on regular basis, indeed. :)
> >
> > But argc == 0 is allowed, so why should git crash?
> 
> Oh, no I was not arguing for making git crash.  I was just
> trying to learn if there is a valid reason to choose to, or
> common misconfiguration that causes it to, run with ac == 0,
> since I did not think of any.

Well, the execlp example is somewhat dragged in. More common case is execvp:

char *argv[] = { NULL }; execvp("git", argv);

The argv array may be empty for various reasons, e.g. pointer arithmetic mistakes.

-- 
ldv

← back to recent threads