Re: [PATCH] 64bit issue in test-parse-options.c

Subsystems: the rest

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] 64bit issue in test-parse-options.c

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:05

Pierre Habouzit [off-list ref] writes:

The proper fix is to let integer be an *INT* (long integer is bogus
anyways) and to put the date in a long using static unsigned long date,
and make OPT_DATE use this long.

I am still puzzled by the original report of the breakage, as H. Merijn
cannot be the first person to ever ran test-parse-options on 64-bit
machine.  Maybe there is a bytesex issue involved as well?

In any case, this should work.

 test-parse-options.c     |    8 +++++---
 t/t0040-parse-options.sh |   11 ++++++++++-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/test-parse-options.c b/test-parse-options.c
index 2a79e72..6e18083 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c

@@ -2,7 +2,8 @@
 #include "parse-options.h"
 
 static int boolean = 0;
-static unsigned long integer = 0;
+static int integer = 0;
+static unsigned long timestamp;
 static int abbrev = 7;
 static int verbose = 0, dry_run = 0, quiet = 0;
 static char *string = NULL;

@@ -32,7 +33,7 @@ int main(int argc, const char **argv)
 		OPT_INTEGER('i', "integer", &integer, "get a integer"),
 		OPT_INTEGER('j', NULL, &integer, "get a integer, too"),
 		OPT_SET_INT(0, "set23", &integer, "set integer to 23", 23),
-		OPT_DATE('t', NULL, &integer, "get timestamp of <time>"),
+		OPT_DATE('t', NULL, &timestamp, "get timestamp of <time>"),
 		OPT_CALLBACK('L', "length", &integer, "str",
 			"get length of <str>", length_callback),
 		OPT_GROUP("String options"),

@@ -56,7 +57,8 @@ int main(int argc, const char **argv)
 	argc = parse_options(argc, argv, options, usage, 0);
 
 	printf("boolean: %d\n", boolean);
-	printf("integer: %lu\n", integer);
+	printf("integer: %u\n", integer);
+	printf("timestamp: %lu\n", timestamp);
 	printf("string: %s\n", string ? string : "(not set)");
 	printf("abbrev: %d\n", abbrev);
 	printf("verbose: %d\n", verbose);

diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 03dbe00..e38241c 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh

@@ -47,6 +47,7 @@ test_expect_success 'test help' '
 cat > expect << EOF
 boolean: 2
 integer: 1729
+timestamp: 0
 string: 123
 abbrev: 7
 verbose: 2

@@ -63,6 +64,7 @@ test_expect_success 'short options' '
 cat > expect << EOF
 boolean: 2
 integer: 1729
+timestamp: 0
 string: 321
 abbrev: 10
 verbose: 2

@@ -88,6 +90,7 @@ test_expect_success 'missing required value' '
 cat > expect << EOF
 boolean: 1
 integer: 13
+timestamp: 0
 string: 123
 abbrev: 7
 verbose: 0

@@ -108,6 +111,7 @@ test_expect_success 'intermingled arguments' '
 cat > expect << EOF
 boolean: 0
 integer: 2
+timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0

@@ -135,6 +139,7 @@ test_expect_success 'ambiguously abbreviated option' '
 cat > expect << EOF
 boolean: 0
 integer: 0
+timestamp: 0
 string: 123
 abbrev: 7
 verbose: 0

@@ -161,6 +166,7 @@ test_expect_success 'detect possible typos' '
 cat > expect <<EOF
 boolean: 0
 integer: 0
+timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0

@@ -177,7 +183,8 @@ test_expect_success 'keep some options as arguments' '
 
 cat > expect <<EOF
 boolean: 0
-integer: 1
+integer: 0
+timestamp: 1
 string: default
 abbrev: 7
 verbose: 0

@@ -197,6 +204,7 @@ cat > expect <<EOF
 Callback: "four", 0
 boolean: 5
 integer: 4
+timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0

@@ -223,6 +231,7 @@ test_expect_success 'OPT_CALLBACK() and callback errors work' '
 cat > expect <<EOF
 boolean: 1
 integer: 23
+timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0

Re: [PATCH] 64bit issue in test-parse-options.c

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:45:05

On Wed, Jul 30, 2008 at 07:11:35PM +0000, Junio C Hamano wrote:

Pierre Habouzit [off-list ref] writes:

quoted

The proper fix is to let integer be an *INT* (long integer is bogus
anyways) and to put the date in a long using static unsigned long date,
and make OPT_DATE use this long.

I am still puzzled by the original report of the breakage, as H. Merijn
cannot be the first person to ever ran test-parse-options on 64-bit
machine.  Maybe there is a bytesex issue involved as well?

  Yes there is: I assume HP-UX works on big endian machines, else on
machines where longs are 64 bits, we put timestamps in it which are
probably current dates, which still fit on 31 (or 32) bits, hence do not
touch the most significant 32 bits hence work on 64 bits little endian
machines (like ... x86_64).

  So when we parse an integer in it again, the 32 most significant bits
remain 0 hence the test passes.

In any case, this should work.

 test-parse-options.c     |    8 +++++---
 t/t0040-parse-options.sh |   11 ++++++++++-
 2 files changed, 15 insertions(+), 4 deletions(-)

  Ack. I wanted to write it but hadn't the time yet, I was just pointing
to the right fix. Thanks for writing the patch :)

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

Re: [PATCH] 64bit issue in test-parse-options.c

From: H.Merijn Brand <hidden>
Date: 2016-06-15 22:45:05

On Wed, 30 Jul 2008 12:11:35 -0700, Junio C Hamano [off-list ref]
wrote:

Pierre Habouzit [off-list ref] writes:

quoted

The proper fix is to let integer be an *INT* (long integer is bogus
anyways) and to put the date in a long using static unsigned long date,
and make OPT_DATE use this long.

I am still puzzled by the original report of the breakage, as H. Merijn
cannot be the first person to ever ran test-parse-options on 64-bit
machine.  Maybe there is a bytesex issue involved as well?

Itanium 2 / 64, and endianness indeed

In any case, this should work.

Yes, that was what I was thinking too, but I had no time to go for the
test cases too. I had it to get to work ASAP.

quoted hunk

 test-parse-options.c     |    8 +++++---
 t/t0040-parse-options.sh |   11 ++++++++++-
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/test-parse-options.c b/test-parse-options.c
index 2a79e72..6e18083 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c

@@ -2,7 +2,8 @@
 #include "parse-options.h"
 
 static int boolean = 0;
-static unsigned long integer = 0;
+static int integer = 0;
+static unsigned long timestamp;
 static int abbrev = 7;
 static int verbose = 0, dry_run = 0, quiet = 0;
 static char *string = NULL;

@@ -32,7 +33,7 @@ int main(int argc, const char **argv)
 		OPT_INTEGER('i', "integer", &integer, "get a integer"),
 		OPT_INTEGER('j', NULL, &integer, "get a integer, too"),
 		OPT_SET_INT(0, "set23", &integer, "set integer to 23", 23),
-		OPT_DATE('t', NULL, &integer, "get timestamp of <time>"),
+		OPT_DATE('t', NULL, &timestamp, "get timestamp of <time>"),
 		OPT_CALLBACK('L', "length", &integer, "str",
 			"get length of <str>", length_callback),
 		OPT_GROUP("String options"),

@@ -56,7 +57,8 @@ int main(int argc, const char **argv)
 	argc = parse_options(argc, argv, options, usage, 0);
 
 	printf("boolean: %d\n", boolean);
-	printf("integer: %lu\n", integer);
+	printf("integer: %u\n", integer);
+	printf("timestamp: %lu\n", timestamp);
 	printf("string: %s\n", string ? string : "(not set)");
 	printf("abbrev: %d\n", abbrev);
 	printf("verbose: %d\n", verbose);

diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 03dbe00..e38241c 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh

@@ -47,6 +47,7 @@ test_expect_success 'test help' '
 cat > expect << EOF
 boolean: 2
 integer: 1729
+timestamp: 0
 string: 123
 abbrev: 7
 verbose: 2

@@ -63,6 +64,7 @@ test_expect_success 'short options' '
 cat > expect << EOF
 boolean: 2
 integer: 1729
+timestamp: 0
 string: 321
 abbrev: 10
 verbose: 2

@@ -88,6 +90,7 @@ test_expect_success 'missing required value' '
 cat > expect << EOF
 boolean: 1
 integer: 13
+timestamp: 0
 string: 123
 abbrev: 7
 verbose: 0

@@ -108,6 +111,7 @@ test_expect_success 'intermingled arguments' '
 cat > expect << EOF
 boolean: 0
 integer: 2
+timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0

@@ -135,6 +139,7 @@ test_expect_success 'ambiguously abbreviated option' '
 cat > expect << EOF
 boolean: 0
 integer: 0
+timestamp: 0
 string: 123
 abbrev: 7
 verbose: 0

@@ -161,6 +166,7 @@ test_expect_success 'detect possible typos' '
 cat > expect <<EOF
 boolean: 0
 integer: 0
+timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0

@@ -177,7 +183,8 @@ test_expect_success 'keep some options as arguments' '
 
 cat > expect <<EOF
 boolean: 0
-integer: 1
+integer: 0
+timestamp: 1
 string: default
 abbrev: 7
 verbose: 0

@@ -197,6 +204,7 @@ cat > expect <<EOF
 Callback: "four", 0
 boolean: 5
 integer: 4
+timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0

@@ -223,6 +231,7 @@ test_expect_success 'OPT_CALLBACK() and callback errors work' '
 cat > expect <<EOF
 boolean: 1
 integer: 23
+timestamp: 0
 string: (not set)
 abbrev: 7
 verbose: 0


-- 
H.Merijn Brand          Amsterdam Perl Mongers  http://amsterdam.pm.org/
using & porting perl 5.6.2, 5.8.x, 5.10.x, 5.11.x on HP-UX 10.20, 11.00,
11.11, 11.23, and 11.31, SuSE 10.1, 10.2, and 10.3, AIX 5.2, and Cygwin.
http://mirrors.develooper.com/hpux/           http://www.test-smoke.org/
http://qa.perl.org      http://www.goldmark.org/jeff/stupid-disclaimers/

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help