Re: [PATCH] MSVC: fix t0040-parse-options
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 28, 2014, 18:19 UTC
- Message-ID
- <xmqq7g7eb2zv.fsf@gitster.dls.corp.google.com>
- In-Reply-To
- <1396008298-1434-1-git-send-email-marat@slonopotamus.org>
Marat Radchenko <marat@slonopotamus.org> writes:
Show 14 quoted lines
> Signed-off-by: Marat Radchenko <marat@slonopotamus.org> > --- > test-parse-options.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/test-parse-options.c b/test-parse-options.c > index 434e8b8..7840493 100644 > --- a/test-parse-options.c > +++ b/test-parse-options.c > @@ -11,6 +11,7 @@ static char *string = NULL; > static char *file = NULL; > static int ambiguous; > static struct string_list list; > +static const char *default_string = "default";
That wastes 4 or 8 bytes compared to
static const char default_string[] = "default";
no?
Show 11 quoted lines
> static int length_callback(const struct option *opt, const char *arg, int unset)
> {
> @@ -60,7 +61,7 @@ int main(int argc, char **argv)
> OPT_STRING('o', NULL, &string, "str", "get another string"),
> OPT_NOOP_NOARG(0, "obsolete"),
> OPT_SET_PTR(0, "default-string", &string,
> - "set string to default", (unsigned long)"default"),
> + "set string to default", default_string),
> OPT_STRING_LIST(0, "list", &list, "str", "add str to list"),
> OPT_GROUP("Magic arguments"),
> OPT_ARGUMENT("quux", "means --quux"),I can see how this patch would not hurt, but at the same time, I cannot see why this patch is a "FIX". A string literal "default" is a pointer to constant string, and being able to cast a pointer to "unsigned long" is something that is done fairly commonly without problems [*1*]. It needs to be explained why this change is needed along the lines of...
We prepare an element in an array of "struct option" with OPT_SET_PTR to point a variable to a literal string "default", but MSVC compiler fails to distim the doshes for such and such reasons.
Work it around by moving the literal string outside the definition of the struct option, which MSVC can understand it.
in the log message.
[Footnote]
*1* The cast should actually be intptr_t for it to be kosher. I
also suspect that the cast should happen inside OPT_SET_PTR()
macro defintion, like in the attached patch.parse-options.h | 2 +- test-parse-options.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/parse-options.h b/parse-options.h index d670cb9..7a24d2e 100644 --- a/parse-options.h +++ b/parse-options.h @@ -129,7 +129,7 @@ struct option { #define OPT_HIDDEN_BOOL(s, l, v, h) { OPTION_SET_INT, (s), (l), (v), NULL, \ (h), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1} #define OPT_SET_PTR(s, l, v, h, p) { OPTION_SET_PTR, (s), (l), (v), NULL, \ - (h), PARSE_OPT_NOARG, NULL, (p) } + (h), PARSE_OPT_NOARG, NULL, (intptr_t)(p) } #define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \ (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) } #define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_("n"), (h) } diff --git a/test-parse-options.c b/test-parse-options.c index 434e8b8..10da63e 100644 --- a/test-parse-options.c +++ b/test-parse-options.c @@ -60,7 +60,7 @@ int main(int argc, char **argv) OPT_STRING('o', NULL, &string, "str", "get another string"), OPT_NOOP_NOARG(0, "obsolete"), OPT_SET_PTR(0, "default-string", &string, - "set string to default", (unsigned long)"default"), + "set string to default", "default"), OPT_STRING_LIST(0, "list", &list, "str", "add str to list"), OPT_GROUP("Magic arguments"), OPT_ARGUMENT("quux", "means --quux"),