Re: [PATCH] AIX 5.2 - bug with 1.6.2.1

Re: [PATCH] AIX 5.2 - bug with 1.6.2.1

[Jeff King]

[I'm cc'ing the git-list; please send bug reports there]

On Tue, Mar 31, 2009 at 01:47:47AM +0200, Pierre Poissinger wrote:

> Looks like the patch posted here
> http://osdir.com/ml/git/2009-01/msg01128.html
> (around d8e96fd86d415554a9c2e09ffb929a9e22fdad25 I think - too late to
> look for it - love google that gave me this guy as first hit...)
> 
> breaks git on AIX 5.2 with gcc 2.9 :-(

Wow, I didn't know people were still using the 2.9 branch of gcc.

> Nothing new to pack.
> trace: built-in: git 'prune-packed'
> The end...
> fatal: unable to run 'git-repack'
> error: waitpid 155256: code 128 (Error 0)
> error: failed to run repack

Hmm. Can you confirm the status being passed back by run_command? The
following patch should do it:

diff --git a/git.c b/git.c
index c2b181e..a99c31b 100644
--- a/git.c
+++ b/git.c
@@ -413,6 +413,7 @@ static void execv_dashed_external(const char **argv)
 	 * OK to return. Otherwise, we just pass along the status code.
 	 */
 	status = run_command_v_opt(argv, 0);
+	fprintf(stderr, "run_command produced status %d\n", status);
 	if (status != -ERR_RUN_COMMAND_EXEC) {
 		if (IS_RUN_COMMAND_ERR(status))
 			die("unable to run '%s'", argv[0]);

> The bottom line: Looks like my box really don't like the run-command
> error define as:
> #define IS_RUN_COMMAND_ERR(x) ((x) <= -ERR_RUN_COMMAND_FORK)
> 
> it will trigger even with x==0 :-)

That seems very wrong. I wonder if it is a problem with the signedness
of enums in that version of gcc. Can you run the following program and
report on its output?

-- >8 --
#include <stdio.h>
enum { FOO = 10000 };
int main()
{
  printf("-FOO: %d\n", -FOO);
  printf("0 <= -FOO: %d\n", 0 <= -FOO);
  printf("-10000 <= -FOO: %d\n", -10000 <= -FOO);
  return 0;
}
-- 8< --

> So, following patch simply change this define to
> #define IS_RUN_COMMAND_ERR(x) ((x) > ERR_RUN_COMMAND_FORK)

That's not right; x is going to be a large negative value. You could
try:

  #define IS_RUN_COMMAND_ERR(x) ((-x) > ERR_RUN_COMMAND_FORK)

The other option is to rework run_command to just return positive values
(which should be fine as long as they remain out of the range of normal
exit codes).

-Peff

Re: [PATCH] AIX 5.2 - bug with 1.6.2.1

[Pierre Poissinger]

On Tue, Mar 31, 2009 at 12:37 PM, Jeff King <peff@peff.net> wrote:
> [I'm cc'ing the git-list; please send bug reports there]
[snip]
>> breaks git on AIX 5.2 with gcc 2.9 :-(
>
> Wow, I didn't know people were still using the 2.9 branch of gcc.

Yep, that or "C for AIX Compiler, Version 6" but this one is awol for git
[Hey, it feels cutting edge - I use Emacs 19 and digital C daily on
OpenVMS 7.1...]

> Hmm. Can you confirm the status being passed back by run_command?
>[snip]
0 - that's why I found out the funny enum stuff...

> That seems very wrong. I wonder if it is a problem with the signedness
> of enums in that version of gcc. Can you run the following program and
> report on its output?
Not only to you... just that's what I noticed...

> -- >8 --
-FOO: -10000
0 <= -FOO: 1
-10000 <= -FOO: 1

This seems to be a "Old GCC'ism" - XLC (version 6...cannot even
understand git code) does it right:
-FOO: -10000
0 <= -FOO: 0
-10000 <= -FOO: 1

>  #define IS_RUN_COMMAND_ERR(x) ((-x) > ERR_RUN_COMMAND_FORK)
oops... works for me with
#define IS_RUN_COMMAND_ERR(x) (-(x) > ERR_RUN_COMMAND_FORK)

> The other option is to rework run_command to just return positive values
> (which should be fine as long as they remain out of the range of normal
> exit codes).
Change define good enough for me and my oldies...

Regs,
Pierre,
-- 
>>> horsemen = ['war', 'pestilence', 'famine']
>>> horsemen.append('Powerbuilder')

Re: [PATCH] AIX 5.2 - bug with 1.6.2.1

[Jeff King]

On Tue, Mar 31, 2009 at 02:20:17PM +0200, Pierre Poissinger wrote:

> >  #define IS_RUN_COMMAND_ERR(x) ((-x) > ERR_RUN_COMMAND_FORK)
> oops... works for me with
> #define IS_RUN_COMMAND_ERR(x) (-(x) > ERR_RUN_COMMAND_FORK)

Oops, it should actually be "-(x) >= ERR_RUN_COMMAND_FORK". But it
shouldn't make a difference in your test, since x was '0' in your case.

Junio, I think this patch should take care of it.

-- >8 --
Subject: [PATCH] fix portability problem with IS_RUN_COMMAND_ERR

Some old versions of gcc don't seem to like us negating an
enum constant. Let's work around it by negating the other
half of the comparison instead.

Reported by Pierre Poissinger on gcc 2.9.
---
 run-command.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/run-command.h b/run-command.h
index 15e870a..e345502 100644
--- a/run-command.h
+++ b/run-command.h
@@ -10,7 +10,7 @@ enum {
 	ERR_RUN_COMMAND_WAITPID_SIGNAL,
 	ERR_RUN_COMMAND_WAITPID_NOEXIT,
 };
-#define IS_RUN_COMMAND_ERR(x) ((x) <= -ERR_RUN_COMMAND_FORK)
+#define IS_RUN_COMMAND_ERR(x) (-(x) >= ERR_RUN_COMMAND_FORK)
 
 struct child_process {
 	const char **argv;
-- 
1.6.2.1.591.geb450

Re: [PATCH] AIX 5.2 - bug with 1.6.2.1

[Pierre Poissinger]

> Junio, I think this patch should take care of it.
Thanks !

Pierre,
-- 
>>> horsemen = ['war', 'pestilence', 'famine']
>>> horsemen.append('Powerbuilder')