threads / patch / 18657

patchRe: [PATCH] AIX 5.2 - bug with 1.6.2.1

Subject: Re: [PATCH] AIX 5.2 - bug with 1.6.2.1

## tl;dr

4 messages between Mar 31, 2009 and Mar 31, 2009. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Jeff King· Mar 31, 2009, 10:37 UTC · lore
[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:
Show 6 quoted lines
> 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.
Show 6 quoted lines
> 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:

Show changes to git.c +1 −3
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
Pierre Poissinger· Mar 31, 2009, 12:20 UTC · re: Jeff King · lore
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')
Jeff King· Mar 31, 2009, 12:29 UTC · re: Pierre Poissinger · lore
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(-)
Show changes to run-command.h +1 −1
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

← back to recent threads