Re: [PATCH] Spell __attribute__ correctly in cache.h.
Martijn Kuipers wrote: [EMAIL PROTECTED]:~$ 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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Spell __attribute__ correctly in cache.h.
Jason Riedy [EMAIL PROTECTED] 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.. - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Spell __attribute__ correctly in cache.h.
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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Spell __attribute__ correctly in cache.h.
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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Spell __attribute__ correctly in cache.h.
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:
[EMAIL PROTECTED]:~$ 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;
}
[EMAIL PROTECTED]:~$ 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
[EMAIL PROTECTED]:~$ gcc --std=c99 -pedantic -Wall -W -ofoo foo.c
[EMAIL PROTECTED]:~$ ./foo
sizeof(struct foo) = 4
sizeof(struct bar) = 8
sizeof(int) = 4
[EMAIL PROTECTED]:~$
(Tested with both 3.3 and 4.0 in Debian unstable.)
--
Antti-Juhani
-
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Spell __attribute__ correctly in cache.h.
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 [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Spell __attribute__ correctly in cache.h.
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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Spell __attribute__ correctly in cache.h.
Jason Riedy [EMAIL PROTECTED] 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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Spell __attribute__ correctly in cache.h.
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 - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Spell __attribute__ correctly in cache.h.
Jason Riedy [EMAIL PROTECTED] 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. - To unsubscribe from this list: send the line unsubscribe git in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
