{"thread":{"id":"1587","subject":"[PATCH] Spell __attribute__ correctly in cache.h.","startedAt":"2005-08-19T04:10:08Z","lastAt":"2005-08-29T08:55:22Z","messageCount":13,"participants":["Jason Riedy","Junio C Hamano","Linus Torvalds","Antti-Juhani Kaijanaho","Martijn Kuipers"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"7544","messageId":"812.1124424608@lotus.CS.Berkeley.EDU","threadId":"1587","inReplyTo":null,"subject":"[PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Jason Riedy","fromEmail":"ejr@cs.berkeley.edu","sentAt":"2005-08-19T04:10:08Z","receivedAt":"2005-08-19T04:10:08Z","isPatch":true,"sender":{"key":"ejr@eecs.berkeley.edu","avatar":"https://gravatar.com/avatar/547fa56f887cab01599edab4e9f813c949c1269e02714f20e0496c56185d9837?d=mp&s=160"},"body":"Sun's cc doesn't know __attribute__.\n\nSigned-off-by: Jason Riedy <ejr@cs.berkeley.edu>\n---\n\n cache.h |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\n4181b19f615b3d56f9fae5f3accd435480aa7d2f\ndiff --git a/cache.h b/cache.h\n--- a/cache.h\n+++ b/cache.h\n@@ -41,7 +41,7 @@\n #endif\n \n #ifndef __attribute__\n-#define __attribute(x)\n+#define __attribute__(x)\n #endif\n \n /*\n"},{"id":"7557","messageId":"7vk6ii1emh.fsf@assigned-by-dhcp.cox.net","threadId":"1587","inReplyTo":"812.1124424608@lotus.CS.Berkeley.EDU","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-19T09:04:54Z","receivedAt":"2005-08-19T09:04:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jason Riedy <ejr@cs.berkeley.edu> writes:\n\n> Sun's cc doesn't know __attribute__.\n\nIt turns out that your patch breaks GCC build (#ifndef\n__attribute__ is true there, and it should be---what it does\ncannot be done in preprocessor alone).  I am going to work it\naround like this.  Could you try it with Sun cc please?\n\n---\ndiff --git a/cache.h b/cache.h\n--- a/cache.h\n+++ b/cache.h\n@@ -38,11 +38,10 @@\n #define NORETURN __attribute__((__noreturn__))\n #else\n #define NORETURN\n-#endif\n-\n #ifndef __attribute__\n #define __attribute__(x)\n #endif\n+#endif\n \n /*\n  * Intensive research over the course of many years has shown that\n"},{"id":"7562","messageId":"4091.1124463516@lotus.CS.Berkeley.EDU","threadId":"1587","inReplyTo":"7vk6ii1emh.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Jason Riedy","fromEmail":"ejr@eecs.berkeley.edu","sentAt":"2005-08-19T14:58:36Z","receivedAt":"2005-08-19T14:58:36Z","isPatch":true,"sender":{"key":"ejr@eecs.berkeley.edu","avatar":"https://gravatar.com/avatar/547fa56f887cab01599edab4e9f813c949c1269e02714f20e0496c56185d9837?d=mp&s=160"},"body":"And Junio C Hamano writes:\n - It turns out that your patch breaks GCC build \n\nWhoops, sorry.  Your fix works with Sun's cc.  \n\nBTW, how would people feel about replacing the \nsetenv() and unsetenv() calls with the older putenv()?\nThe Solaris version I have to work on doesn't have \nthe nicer functions (and I'm not an admin).  I have \nto check that the unsetenv() in git-fsck-cache.c works \ncorrectly as a putenv before I send along a patch.  \nThere's also the issue that /bin/sh isn't bash, but an \ninstallation-time helper script can fix that.\n\nJason\n"},{"id":"7574","messageId":"7v64u1ya7c.fsf@assigned-by-dhcp.cox.net","threadId":"1587","inReplyTo":"4091.1124463516@lotus.CS.Berkeley.EDU","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-19T19:53:59Z","receivedAt":"2005-08-19T19:53:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jason Riedy <ejr@EECS.Berkeley.EDU> writes:\n\n> And Junio C Hamano writes:\n>  - It turns out that your patch breaks GCC build \n>\n> Whoops, sorry.  Your fix works with Sun's cc.  \n\nThanks.\n\n> BTW, how would people feel about replacing the \n> setenv() and unsetenv() calls with the older putenv()?\n> The Solaris version I have to work on doesn't have \n> the nicer functions (and I'm not an admin).  I have \n> to check that the unsetenv() in git-fsck-cache.c works \n> correctly as a putenv before I send along a patch.  \n\nNo comment on this one at this moment until I do my own digging\na bit.\n\n> There's also the issue that /bin/sh isn't bash, but an \n> installation-time helper script can fix that.\n\nMy personal preference is to rewrite parts that are easily\nunbashified first before going that route, but I suspect that it\nwould end up being the best practical solution to simply admit\nthat we depend on bash, start our scripts with \"#!/bin/bash\",\nand rewrite them \"#!/usr/local/bin/bash\" upon installation;\nmodulo that it may be a stupid and ugly workaround.\n"},{"id":"7668","messageId":"2655.1124832058@lotus.CS.Berkeley.EDU","threadId":"1587","inReplyTo":"7v64u1ya7c.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Jason Riedy","fromEmail":"ejr@eecs.berkeley.edu","sentAt":"2005-08-23T21:20:58Z","receivedAt":"2005-08-23T21:20:58Z","isPatch":true,"sender":{"key":"ejr@eecs.berkeley.edu","avatar":"https://gravatar.com/avatar/547fa56f887cab01599edab4e9f813c949c1269e02714f20e0496c56185d9837?d=mp&s=160"},"body":"And Junio C Hamano writes:\n - > BTW, how would people feel about replacing the \n - > setenv() and unsetenv() calls with the older putenv()?\n - No comment on this one at this moment until I do my own digging\n - a bit.\n\nIf you're interested, I have a few patches in\n  http://www.cs.berkeley.edu/~ejr/gits/git.git#portable\nthat let git compile with xlc on AIX and Sun's non-c99 \ncc on Solaris.  Changes:\n +    Replace C99 array initializers with code.\n +    Replace unsetenv() and setenv() with older putenv().\n +    Include sys/time.h in daemon.c.\n +    Fix ?: statements.\n +    Replace zero-length array decls with [].\nThe top two may or may not be acceptable.  The third may\nnot be necessary if I can find the right -Ds for fd_set.\nThe last two just remove GNU C extensions.  Makefile \nchanges (including extra -Ds for features) not included, \nbut could be.  Tell me if you want any of these mailed.\n\nNot all the tests pass on non-Linux, but I won't have time \nto look at them for a bit.  With the GNU findutils and \ncoreutils, it works well enough for basic use.  The failing \ntests might be from using non-GNU utilities.  Rooting out\nall the dependencies is a tad painful.\n\nJason\n"},{"id":"7863","messageId":"7vll2mmkqk.fsf@assigned-by-dhcp.cox.net","threadId":"1587","inReplyTo":"2655.1124832058@lotus.CS.Berkeley.EDU","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-08-28T10:14:27Z","receivedAt":"2005-08-28T10:14:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jason Riedy <ejr@EECS.Berkeley.EDU> writes:\n\n> If you're interested, I have a few patches in\n>   http://www.cs.berkeley.edu/~ejr/gits/git.git#portable\n> that let git compile with xlc on AIX and Sun's non-c99 \n> cc on Solaris.\n\nI've taken a look at them.  Thanks.\n\n> Changes:\n>  +    Replace C99 array initializers with code.\n\nI presume this is to help older compilers?\n\n>  +    Replace unsetenv() and setenv() with older putenv().\n\nI wonder how buggy various implementations of\nputenv(\"THIS_ENV_VAR\") are to remove the variable.\n\n>  +    Include sys/time.h in daemon.c.\n>  +    Fix ?: statements.\n\nI do not have much problem with these two.\n\n>  +    Replace zero-length array decls with [].\n\nThis I am ambivalent about.  If we are just trying to help older\ncompilers (see your \"array initializers\" patch), we should be\ndoing C90 way of \"array[1]\" and teach users to subtract 1 from\nthe allocate count.  While I do not have much objection against\nusing C99 flexible array member notation, I wonder how people\nfind being able to compile with older compilers a major issue..\n"},{"id":"7876","messageId":"19723.1125249085@lotus.CS.Berkeley.EDU","threadId":"1587","inReplyTo":"7vll2mmkqk.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Jason Riedy","fromEmail":"ejr@eecs.berkeley.edu","sentAt":"2005-08-28T17:11:25Z","receivedAt":"2005-08-28T17:11:25Z","isPatch":true,"sender":{"key":"ejr@eecs.berkeley.edu","avatar":"https://gravatar.com/avatar/547fa56f887cab01599edab4e9f813c949c1269e02714f20e0496c56185d9837?d=mp&s=160"},"body":"And Junio C Hamano writes:\n - >  +    Replace C99 array initializers with code.\n - I presume this is to help older compilers?\n\nYes, so it's relatively unimportant.  I could work\naround it in my situation; I only included it \nbecause it's \"necessary\" for some Sun compilers on\nolder Solaris installations.  A static gcc build\nworks well enough in my situation.\n\n - >  +    Replace unsetenv() and setenv() with older putenv().\n - I wonder how buggy various implementations of\n - putenv(\"THIS_ENV_VAR\") are to remove the variable.\n\nI don't know, and it doesn't seem to matter in the git \ncode.  I didn't see checks for existance, but I may have\nmissed something.  Most uses replace a NULL with a \npointer to \"\", which is why I just used putenv(\"FOO=\").\n\nThis is to cope with an older Solaris installation I \nhave to use.  ugh.  I can use a better compiler, but \nI'm stuck with the system library.  Other older systems\nprobably don't have unsetenv(), either.\n\nThe \"right\" way would be to twiddle the environ and \nuse execle, but that's nasty.\n\n - >  +    Replace zero-length array decls with [].\n - This I am ambivalent about.\n\nI'm fine with requiring a limited C99 compiler.  A\npedantic compiler will reject members with a length\nof zero.  6.7.5.2 para1 requires a value greater than\nzero for a constant array size.  So the code now (with\n[0] decls) is neither C89 nor C99.\n\nJason\n"},{"id":"7877","messageId":"Pine.LNX.4.58.0508281045060.3317@g5.osdl.org","threadId":"1587","inReplyTo":"19723.1125249085@lotus.CS.Berkeley.EDU","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-28T17:46:58Z","receivedAt":"2005-08-28T17:46:58Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 28 Aug 2005, Jason Riedy wrote:\n> \n> I'm fine with requiring a limited C99 compiler.  A\n> pedantic compiler will reject members with a length\n> of zero.  6.7.5.2 para1 requires a value greater than\n> zero for a constant array size.  So the code now (with\n> [0] decls) is neither C89 nor C99.\n\nBut using \"array[]\" means that \"sizeof()\" no longer works, and then you \nhave to use \"offsetof()\", which is a big pain.\n\nI think all relevant compilers end up accepting [0] (probably giving a\nwarning, especially in some pedantic mode), since it's been how gcc users\nhave been doing things for years (gcc was late to the [] syntax).\n\n\t\t\tLinus\n"},{"id":"7878","messageId":"43120BC5.8060608@kaijanaho.info","threadId":"1587","inReplyTo":"Pine.LNX.4.58.0508281045060.3317@g5.osdl.org","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Antti-Juhani Kaijanaho","fromEmail":"antti-juhani@kaijanaho.info","sentAt":"2005-08-28T19:08:53Z","receivedAt":"2005-08-28T19:08:53Z","isPatch":true,"sender":{"key":"antti-juhani@kaijanaho.info","avatar":"https://gravatar.com/avatar/d18220c6252159e5b879101731700abcdc16af1fa9bc34d0ed13e2f35f87e222?d=mp&s=160"},"body":"Linus Torvalds wrote:\n> But using \"array[]\" means that \"sizeof()\" no longer works, and then you \n> have to use \"offsetof()\", which is a big pain.\n\nThis is not true under C99.  If an array[] is the last member of a\nstruct (which is what we are, AFAIK, talking about), then sizeof that\nstruct is defined and gives the size of that struct as if the array's\nsize were zero (but the struct cannot be used in an automatic context).\n Hence the idiom\n\n  struct foo {\n    ...\n    int bar[];\n  };\n  ...\n  struct foo *baz = malloc(sizeof *baz + 15 * sizeof (int));\n  ...\n\nwhich allocates baz in such a way that bar is a 15-element array of ints.\n\nOf course, I cannot speak of how other non-C99 compilers implement this,\nbut GCC gets it right:\n\najk@kukkamaljakko:~$ cat foo.c\n#include <stdio.h>\n\nstruct foo {\n        int a;\n        int b[];\n};\n\nstruct bar {\n        int a;\n        int b[1];\n};\n\nint main(void)\n{\n        printf(\"sizeof(struct foo) = %zu\\n\"\n               \"sizeof(struct bar) = %zu\\n\"\n               \"sizeof(int) = %zu\\n\",\n               sizeof(struct foo),\n               sizeof(struct bar),\n               sizeof(int));\n        return 0;\n}\najk@kukkamaljakko:~$ gcc --std=c89 -pedantic -Wall -W -ofoo foo.c\nfoo.c:5: warning: ISO C90 does not support flexible array members\nfoo.c: In function 'main':\nfoo.c:20: warning: ISO C90 does not support the 'z' printf length modifier\nfoo.c:20: warning: ISO C90 does not support the 'z' printf length modifier\nfoo.c:20: warning: ISO C90 does not support the 'z' printf length modifier\najk@kukkamaljakko:~$ gcc --std=c99 -pedantic -Wall -W -ofoo foo.c\najk@kukkamaljakko:~$ ./foo\nsizeof(struct foo) = 4\nsizeof(struct bar) = 8\nsizeof(int) = 4\najk@kukkamaljakko:~$\n\n(Tested with both 3.3 and 4.0 in Debian unstable.)\n-- \nAntti-Juhani\n"},{"id":"7879","messageId":"Pine.LNX.4.58.0508281245150.3317@g5.osdl.org","threadId":"1587","inReplyTo":"43120BC5.8060608@kaijanaho.info","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2005-08-28T19:48:02Z","receivedAt":"2005-08-28T19:48:02Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sun, 28 Aug 2005, Antti-Juhani Kaijanaho wrote:\n> \n> This is not true under C99.  If an array[] is the last member of a\n> struct (which is what we are, AFAIK, talking about), then sizeof that\n> struct is defined and gives the size of that struct as if the array's\n> size were zero (but the struct cannot be used in an automatic context).\n\nAhh, thanks. Mea culpa, I thought it was illegal in general. In that case,\nthe only reason not to use [] is that older gcc's don't like it, but even\nthat version cut-off may be old enough to not matter.\n\nAnybody know? gcc-2.95 is still considered production at least for the\nkernel. I don't have it available to test whether it understands []\nthough.\n\n\t\tLinus\n"},{"id":"7891","messageId":"4312C492.2010307@gmail.com","threadId":"1587","inReplyTo":"Pine.LNX.4.58.0508281245150.3317@g5.osdl.org","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Martijn Kuipers","fromEmail":"martijn.kuipers@gmail.com","sentAt":"2005-08-29T08:17:22Z","receivedAt":"2005-08-29T08:17:22Z","isPatch":true,"sender":{"key":"martijn.kuipers@gmail.com","avatar":"https://gravatar.com/avatar/bab8b7fab945f46d5a7c38c57f83e4d8608b0818246c99598028f4eadd4e17e1?d=mp&s=160"},"body":"Hi,\n\nI still had 2.95 on my machine (Debian). Results are:\n\nmartijn@hobbes:~$ gcc-2.95 --version\n2.95.4\n\nmartijn@hobbes:~$ gcc-2.95 --std=c99 -pedantic -Wall -W -ofoo foo.c\ncc1: unknown C standard `c99'\nfoo.c:5: field `b' has incomplete type\nfoo.c: In function `main':\nfoo.c:20: warning: unknown conversion type character `z' in format\nfoo.c:20: warning: unknown conversion type character `z' in format\nfoo.c:20: warning: unknown conversion type character `z' in format\nfoo.c:20: warning: too many arguments for format\n\nKind regards,\nMartijn\n\nLinus Torvalds wrote:\n\n>On Sun, 28 Aug 2005, Antti-Juhani Kaijanaho wrote:\n>  \n>\n>>This is not true under C99.  If an array[] is the last member of a\n>>struct (which is what we are, AFAIK, talking about), then sizeof that\n>>struct is defined and gives the size of that struct as if the array's\n>>size were zero (but the struct cannot be used in an automatic context).\n>>    \n>>\n>\n>Ahh, thanks. Mea culpa, I thought it was illegal in general. In that case,\n>the only reason not to use [] is that older gcc's don't like it, but even\n>that version cut-off may be old enough to not matter.\n>\n>Anybody know? gcc-2.95 is still considered production at least for the\n>kernel. I don't have it available to test whether it understands []\n>though.\n>\n>\t\tLinus\n>-\n>To unsubscribe from this list: send the line \"unsubscribe git\" in\n>the body of a message to majordomo@vger.kernel.org\n>More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n>  \n>\n"},{"id":"7892","messageId":"4312C8D6.9010200@kaijanaho.info","threadId":"1587","inReplyTo":"4312C492.2010307@gmail.com","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Antti-Juhani Kaijanaho","fromEmail":"antti-juhani@kaijanaho.info","sentAt":"2005-08-29T08:35:34Z","receivedAt":"2005-08-29T08:35:34Z","isPatch":true,"sender":{"key":"antti-juhani@kaijanaho.info","avatar":"https://gravatar.com/avatar/d18220c6252159e5b879101731700abcdc16af1fa9bc34d0ed13e2f35f87e222?d=mp&s=160"},"body":"Martijn Kuipers wrote:\n> martijn@hobbes:~$ gcc-2.95 --std=c99 -pedantic -Wall -W -ofoo foo.c\n> cc1: unknown C standard `c99'\n\nThis makes this test a little less useful.  Try with --std=c9x (GCC 2.95\nis old enough not to know the standard by the \"official\" name).\n\nAccording to GCC 3.0 C99 status page [1], 3.0 supported flexible array\nmembers.  There is no similar page for 2.95.\n\n-- \nAntti-Juhani\n\n[1] http://gcc.gnu.org/gcc-3.0/c99status.html\n"},{"id":"7893","messageId":"4312CD7A.4010401@gmail.com","threadId":"1587","inReplyTo":"4312C8D6.9010200@kaijanaho.info","subject":"Re: [PATCH] Spell __attribute__ correctly in cache.h.","fromName":"Martijn Kuipers","fromEmail":"martijn.kuipers@gmail.com","sentAt":"2005-08-29T08:55:22Z","receivedAt":"2005-08-29T08:55:22Z","isPatch":true,"sender":{"key":"martijn.kuipers@gmail.com","avatar":"https://gravatar.com/avatar/bab8b7fab945f46d5a7c38c57f83e4d8608b0818246c99598028f4eadd4e17e1?d=mp&s=160"},"body":"Sorry, I am not very familiar with all those command line options:\nAnyway, redo was easy:\nwith: --std=c9x\n\ngcc-2.95 --std=c9x -pedantic -Wall -W -ofoo foo.c\nfoo.c:5: field `b' has incomplete type\nfoo.c: In function `main':\nfoo.c:20: warning: unknown conversion type character `z' in format\nfoo.c:20: warning: unknown conversion type character `z' in format\nfoo.c:20: warning: unknown conversion type character `z' in format\nfoo.c:20: warning: too many arguments for format\n\nI found one man-page online \n(http://gcc.gnu.org/onlinedocs/gcc-2.95.3/gcc_2.html#SEC3), which seemed \nto suggest -fstd=c9x (if I understood correclty). This gave the same \nresults.\n\nI hope this makes it a bit more useful :-)\nIf not, just let me know.\n\nKind regards,\nMartijn\n\n\nAntti-Juhani Kaijanaho wrote:\n\n>Martijn Kuipers wrote:\n>  \n>\n>>martijn@hobbes:~$ gcc-2.95 --std=c99 -pedantic -Wall -W -ofoo foo.c\n>>cc1: unknown C standard `c99'\n>>    \n>>\n>\n>This makes this test a little less useful.  Try with --std=c9x (GCC 2.95\n>is old enough not to know the standard by the \"official\" name).\n>\n>According to GCC 3.0 C99 status page [1], 3.0 supported flexible array\n>members.  There is no similar page for 2.95.\n>\n>  \n>\n"}]}