[PATCH] Spell __attribute__ correctly in cache.h.

[PATCH] Spell __attribute__ correctly in cache.h.

[Jason Riedy]

Sun's cc doesn't know __attribute__.

Signed-off-by: Jason Riedy <ejr@cs.berkeley.edu>
---

 cache.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

4181b19f615b3d56f9fae5f3accd435480aa7d2f
diff --git a/cache.h b/cache.h
--- a/cache.h
+++ b/cache.h
@@ -41,7 +41,7 @@
 #endif
 
 #ifndef __attribute__
-#define __attribute(x)
+#define __attribute__(x)
 #endif
 
 /*

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Junio C Hamano]

Jason Riedy <ejr@cs.berkeley.edu> writes:

> Sun's cc doesn't know __attribute__.

It turns out that your patch breaks GCC build (#ifndef
__attribute__ is true there, and it should be---what it does
cannot be done in preprocessor alone).  I am going to work it
around like this.  Could you try it with Sun cc please?

---
diff --git a/cache.h b/cache.h
--- a/cache.h
+++ b/cache.h
@@ -38,11 +38,10 @@
 #define NORETURN __attribute__((__noreturn__))
 #else
 #define NORETURN
-#endif
-
 #ifndef __attribute__
 #define __attribute__(x)
 #endif
+#endif
 
 /*
  * Intensive research over the course of many years has shown that

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Jason Riedy]

And Junio C Hamano writes:
 - It turns out that your patch breaks GCC build 

Whoops, sorry.  Your fix works with Sun's cc.  

BTW, how would people feel about replacing the 
setenv() and unsetenv() calls with the older putenv()?
The Solaris version I have to work on doesn't have 
the nicer functions (and I'm not an admin).  I have 
to check that the unsetenv() in git-fsck-cache.c works 
correctly as a putenv before I send along a patch.  
There's also the issue that /bin/sh isn't bash, but an 
installation-time helper script can fix that.

Jason

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Junio C Hamano]

Jason Riedy <ejr@EECS.Berkeley.EDU> writes:

> And Junio C Hamano writes:
>  - It turns out that your patch breaks GCC build 
>
> Whoops, sorry.  Your fix works with Sun's cc.  

Thanks.

> BTW, how would people feel about replacing the 
> setenv() and unsetenv() calls with the older putenv()?
> The Solaris version I have to work on doesn't have 
> the nicer functions (and I'm not an admin).  I have 
> to check that the unsetenv() in git-fsck-cache.c works 
> correctly as a putenv before I send along a patch.  

No comment on this one at this moment until I do my own digging
a bit.

> There's also the issue that /bin/sh isn't bash, but an 
> installation-time helper script can fix that.

My personal preference is to rewrite parts that are easily
unbashified first before going that route, but I suspect that it
would end up being the best practical solution to simply admit
that we depend on bash, start our scripts with "#!/bin/bash",
and rewrite them "#!/usr/local/bin/bash" upon installation;
modulo that it may be a stupid and ugly workaround.

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Jason Riedy]

And Junio C Hamano writes:
 - > BTW, how would people feel about replacing the 
 - > setenv() and unsetenv() calls with the older putenv()?
 - No comment on this one at this moment until I do my own digging
 - a bit.

If you're interested, I have a few patches in
  http://www.cs.berkeley.edu/~ejr/gits/git.git#portable
that let git compile with xlc on AIX and Sun's non-c99 
cc on Solaris.  Changes:
 +    Replace C99 array initializers with code.
 +    Replace unsetenv() and setenv() with older putenv().
 +    Include sys/time.h in daemon.c.
 +    Fix ?: statements.
 +    Replace zero-length array decls with [].
The top two may or may not be acceptable.  The third may
not be necessary if I can find the right -Ds for fd_set.
The last two just remove GNU C extensions.  Makefile 
changes (including extra -Ds for features) not included, 
but could be.  Tell me if you want any of these mailed.

Not all the tests pass on non-Linux, but I won't have time 
to look at them for a bit.  With the GNU findutils and 
coreutils, it works well enough for basic use.  The failing 
tests might be from using non-GNU utilities.  Rooting out
all the dependencies is a tad painful.

Jason

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Junio C Hamano]

Jason Riedy <ejr@EECS.Berkeley.EDU> writes:

> If you're interested, I have a few patches in
>   http://www.cs.berkeley.edu/~ejr/gits/git.git#portable
> that let git compile with xlc on AIX and Sun's non-c99 
> cc on Solaris.

I've taken a look at them.  Thanks.

> Changes:
>  +    Replace C99 array initializers with code.

I presume this is to help older compilers?

>  +    Replace unsetenv() and setenv() with older putenv().

I wonder how buggy various implementations of
putenv("THIS_ENV_VAR") are to remove the variable.

>  +    Include sys/time.h in daemon.c.
>  +    Fix ?: statements.

I do not have much problem with these two.

>  +    Replace zero-length array decls with [].

This I am ambivalent about.  If we are just trying to help older
compilers (see your "array initializers" patch), we should be
doing C90 way of "array[1]" and teach users to subtract 1 from
the allocate count.  While I do not have much objection against
using C99 flexible array member notation, I wonder how people
find being able to compile with older compilers a major issue..

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Jason Riedy]

And Junio C Hamano writes:
 - >  +    Replace C99 array initializers with code.
 - I presume this is to help older compilers?

Yes, so it's relatively unimportant.  I could work
around it in my situation; I only included it 
because it's "necessary" for some Sun compilers on
older Solaris installations.  A static gcc build
works well enough in my situation.

 - >  +    Replace unsetenv() and setenv() with older putenv().
 - I wonder how buggy various implementations of
 - putenv("THIS_ENV_VAR") are to remove the variable.

I don't know, and it doesn't seem to matter in the git 
code.  I didn't see checks for existance, but I may have
missed something.  Most uses replace a NULL with a 
pointer to "", which is why I just used putenv("FOO=").

This is to cope with an older Solaris installation I 
have to use.  ugh.  I can use a better compiler, but 
I'm stuck with the system library.  Other older systems
probably don't have unsetenv(), either.

The "right" way would be to twiddle the environ and 
use execle, but that's nasty.

 - >  +    Replace zero-length array decls with [].
 - This I am ambivalent about.

I'm fine with requiring a limited C99 compiler.  A
pedantic compiler will reject members with a length
of zero.  6.7.5.2 para1 requires a value greater than
zero for a constant array size.  So the code now (with
[0] decls) is neither C89 nor C99.

Jason

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Linus Torvalds]

On Sun, 28 Aug 2005, Jason Riedy wrote:
> 
> I'm fine with requiring a limited C99 compiler.  A
> pedantic compiler will reject members with a length
> of zero.  6.7.5.2 para1 requires a value greater than
> zero for a constant array size.  So the code now (with
> [0] decls) is neither C89 nor C99.

But using "array[]" means that "sizeof()" no longer works, and then you 
have to use "offsetof()", which is a big pain.

I think all relevant compilers end up accepting [0] (probably giving a
warning, especially in some pedantic mode), since it's been how gcc users
have been doing things for years (gcc was late to the [] syntax).

			Linus

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Antti-Juhani Kaijanaho]

Linus Torvalds wrote:
> But using "array[]" means that "sizeof()" no longer works, and then you 
> have to use "offsetof()", which is a big pain.

This is not true under C99.  If an array[] is the last member of a
struct (which is what we are, AFAIK, talking about), then sizeof that
struct is defined and gives the size of that struct as if the array's
size were zero (but the struct cannot be used in an automatic context).
 Hence the idiom

  struct foo {
    ...
    int bar[];
  };
  ...
  struct foo *baz = malloc(sizeof *baz + 15 * sizeof (int));
  ...

which allocates baz in such a way that bar is a 15-element array of ints.

Of course, I cannot speak of how other non-C99 compilers implement this,
but GCC gets it right:

ajk@kukkamaljakko:~$ cat foo.c
#include <stdio.h>

struct foo {
        int a;
        int b[];
};

struct bar {
        int a;
        int b[1];
};

int main(void)
{
        printf("sizeof(struct foo) = %zu\n"
               "sizeof(struct bar) = %zu\n"
               "sizeof(int) = %zu\n",
               sizeof(struct foo),
               sizeof(struct bar),
               sizeof(int));
        return 0;
}
ajk@kukkamaljakko:~$ gcc --std=c89 -pedantic -Wall -W -ofoo foo.c
foo.c:5: warning: ISO C90 does not support flexible array members
foo.c: In function 'main':
foo.c:20: warning: ISO C90 does not support the 'z' printf length modifier
foo.c:20: warning: ISO C90 does not support the 'z' printf length modifier
foo.c:20: warning: ISO C90 does not support the 'z' printf length modifier
ajk@kukkamaljakko:~$ gcc --std=c99 -pedantic -Wall -W -ofoo foo.c
ajk@kukkamaljakko:~$ ./foo
sizeof(struct foo) = 4
sizeof(struct bar) = 8
sizeof(int) = 4
ajk@kukkamaljakko:~$

(Tested with both 3.3 and 4.0 in Debian unstable.)
-- 
Antti-Juhani

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Linus Torvalds]

On Sun, 28 Aug 2005, Antti-Juhani Kaijanaho wrote:
> 
> This is not true under C99.  If an array[] is the last member of a
> struct (which is what we are, AFAIK, talking about), then sizeof that
> struct is defined and gives the size of that struct as if the array's
> size were zero (but the struct cannot be used in an automatic context).

Ahh, thanks. Mea culpa, I thought it was illegal in general. In that case,
the only reason not to use [] is that older gcc's don't like it, but even
that version cut-off may be old enough to not matter.

Anybody know? gcc-2.95 is still considered production at least for the
kernel. I don't have it available to test whether it understands []
though.

		Linus

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Martijn Kuipers]

Hi,

I still had 2.95 on my machine (Debian). Results are:

martijn@hobbes:~$ gcc-2.95 --version
2.95.4

martijn@hobbes:~$ gcc-2.95 --std=c99 -pedantic -Wall -W -ofoo foo.c
cc1: unknown C standard `c99'
foo.c:5: field `b' has incomplete type
foo.c: In function `main':
foo.c:20: warning: unknown conversion type character `z' in format
foo.c:20: warning: unknown conversion type character `z' in format
foo.c:20: warning: unknown conversion type character `z' in format
foo.c:20: warning: too many arguments for format

Kind regards,
Martijn

Linus Torvalds wrote:

>On Sun, 28 Aug 2005, Antti-Juhani Kaijanaho wrote:
>  
>
>>This is not true under C99.  If an array[] is the last member of a
>>struct (which is what we are, AFAIK, talking about), then sizeof that
>>struct is defined and gives the size of that struct as if the array's
>>size were zero (but the struct cannot be used in an automatic context).
>>    
>>
>
>Ahh, thanks. Mea culpa, I thought it was illegal in general. In that case,
>the only reason not to use [] is that older gcc's don't like it, but even
>that version cut-off may be old enough to not matter.
>
>Anybody know? gcc-2.95 is still considered production at least for the
>kernel. I don't have it available to test whether it understands []
>though.
>
>		Linus
>-
>To unsubscribe from this list: send the line "unsubscribe git" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>  
>

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Antti-Juhani Kaijanaho]

Martijn Kuipers wrote:
> martijn@hobbes:~$ gcc-2.95 --std=c99 -pedantic -Wall -W -ofoo foo.c
> cc1: unknown C standard `c99'

This makes this test a little less useful.  Try with --std=c9x (GCC 2.95
is old enough not to know the standard by the "official" name).

According to GCC 3.0 C99 status page [1], 3.0 supported flexible array
members.  There is no similar page for 2.95.

-- 
Antti-Juhani

[1] http://gcc.gnu.org/gcc-3.0/c99status.html

Re: [PATCH] Spell __attribute__ correctly in cache.h.

[Martijn Kuipers]

Sorry, I am not very familiar with all those command line options:
Anyway, redo was easy:
with: --std=c9x

gcc-2.95 --std=c9x -pedantic -Wall -W -ofoo foo.c
foo.c:5: field `b' has incomplete type
foo.c: In function `main':
foo.c:20: warning: unknown conversion type character `z' in format
foo.c:20: warning: unknown conversion type character `z' in format
foo.c:20: warning: unknown conversion type character `z' in format
foo.c:20: warning: too many arguments for format

I found one man-page online 
(http://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_2.html#SEC3), which seemed 
to suggest -fstd=c9x (if I understood correclty). This gave the same 
results.

I hope this makes it a bit more useful :-)
If not, just let me know.

Kind regards,
Martijn


Antti-Juhani Kaijanaho wrote:

>Martijn Kuipers wrote:
>  
>
>>martijn@hobbes:~$ gcc-2.95 --std=c99 -pedantic -Wall -W -ofoo foo.c
>>cc1: unknown C standard `c99'
>>    
>>
>
>This makes this test a little less useful.  Try with --std=c9x (GCC 2.95
>is old enough not to know the standard by the "official" name).
>
>According to GCC 3.0 C99 status page [1], 3.0 supported flexible array
>members.  There is no similar page for 2.95.
>
>  
>