Re: [RFC] Re: Convert 'git blame' to parse_options()

From: Pierre Habouzit <madcoder@debian.org>
Date: Jun 23, 2008, 22:36 UTC
Message-ID: <20080623223608.GN13395@artemis.madism.org>
In-Reply-To: <20080623222404.GM13395@artemis.madism.org>

On Mon, Jun 23, 2008 at 10:24:05PM +0000, Pierre Habouzit wrote:

Show 9 quoted lines

> Yeah actually one can use it this way, what I wrote can be used to do
> the very same thing as your _KEEP_UNKNOWN flag, but also more.
>  
> Though I didn't fixed the fact that parse_options clobbers argv[0],
> which can be easily fixed. The issue with that is that _some_ callers
> use the fact that the filtered argv is NULL terminated. I'm unsure if
> posix says that argv[argc] is readable and NULL at all, if it isn't,
> then changing that would break git for some commands on some OSes other
> than Linux and BSD where AFAICT argv[argc] == NULL holds.

Actually one can do it this way without breaking "backward compatibility" here:

----8<---
From 8955adf557fded857eacc6c03c885a3d6334580f Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <madcoder@debian.org>
Date: Tue, 24 Jun 2008 00:31:31 +0200
Subject: [PATCH] parse-opt: add PARSE_OPT_KEEP_ARGV0 parser option.

This way, argv[0] isn't clobbered, to the cost of maybe not having a resulting NULL terminated argv array.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 parse-options.c |    8 +++++---
 parse-options.h |    2 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index 1ade2ac..c1c5a94 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -246,7 +246,7 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
 	memset(ctx, 0, sizeof(*ctx));
 	ctx->argc = argc - 1;
 	ctx->argv = argv + 1;
-	ctx->out  = argv;
+	ctx->out  = argv + ((flags & PARSE_OPT_KEEP_ARGV0) != 0);
 	ctx->flags = flags;
 	strbuf_init(&ctx->buf, 0);
 }
@@ -329,10 +329,12 @@ unknown_fixup:
 
 int parse_options_end(struct parse_opt_ctx_t *ctx)
 {
+	int res = ctx->cpidx + ctx->argc;
 	memmove(ctx->out + ctx->cpidx, ctx->argv, ctx->argc * sizeof(*ctx->out));
-	ctx->out[ctx->cpidx + ctx->argc] = NULL;
+	if (ctx->out + res < ctx->argv + ctx->argc)
+		ctx->out[res] = NULL;
 	strbuf_release(&ctx->buf);
-	return ctx->cpidx + ctx->argc;
+	return res;
 }
 
 int parse_options(int argc, const char **argv, const struct option *options,
diff --git a/parse-options.h b/parse-options.h
index 14447d5..6745c7d 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -22,6 +22,8 @@ enum parse_opt_type {
 enum parse_opt_flags {
 	PARSE_OPT_KEEP_DASHDASH = 1,
 	PARSE_OPT_STOP_AT_NON_OPTION = 2,
+	/* using that option, the filtered argv may not be NULL terminated */
+	PARSE_OPT_KEEP_ARGV0 = 4,
 };
 
 enum parse_opt_option_flags {

-- 
1.5.6.117.g4959d.dirty
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org