{"thread":{"id":"10075","subject":"[PATCH 2/2] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions","startedAt":"2007-09-29T13:00:13Z","lastAt":"2007-09-29T18:15:21Z","messageCount":3,"participants":["Andy Parkins","Junio C Hamano","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"54337","messageId":"200709291400.13880.andyparkins@gmail.com","threadId":"10075","inReplyTo":null,"subject":"[PATCH 2/2] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions","fromName":"Andy Parkins","fromEmail":"andyparkins@gmail.com","sentAt":"2007-09-29T13:00:13Z","receivedAt":"2007-09-29T13:00:13Z","isPatch":true,"sender":{"key":"andyparkins@gmail.com","avatar":null},"body":"Bill Lear pointed out that the following:\n\n #define PATH_REMOTES             \"remotes/\"\n #define STRLEN_PATH_REMOTES      8\n\nCould be replaced by the less error-prone\n\n #define PATH_REMOTES \"remotes/\"\n #define LIT_STRLEN(S) ((sizeof(S) / sizeof(S[0])) -1)\n #define STRLEN_PATH_REMOTES LIT_STRLEN(PATH_REMOTES)\n\nwhich is what this patch does.\n\nSigned-off-by: Andy Parkins <andyparkins@gmail.com>\n---\n refs.h |   11 ++++++-----\n 1 files changed, 6 insertions(+), 5 deletions(-)\n\ndiff --git a/refs.h b/refs.h\nindex 1025d04..fb58889 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -13,16 +13,17 @@ struct ref_lock {\n #define REF_ISSYMREF 01\n #define REF_ISPACKED 02\n \n+#define LIT_STRLEN(S)            ((sizeof(S) / sizeof(S[0])) -1)\n #define PATH_OBJECTS             \"objects/\"\n-#define STRLEN_PATH_OBJECTS      8\n+#define STRLEN_PATH_OBJECTS      LIT_STRLEN(PATH_OBJECTS)\n #define PATH_REFS                \"refs/\"\n-#define STRLEN_PATH_REFS         5\n+#define STRLEN_PATH_REFS         LIT_STRLEN(PATH_REFS)\n #define PATH_HEADS               \"heads/\"\n-#define STRLEN_PATH_HEADS        6\n+#define STRLEN_PATH_HEADS        LIT_STRLEN(PATH_HEADS)\n #define PATH_TAGS                \"tags/\"\n-#define STRLEN_PATH_TAGS         5\n+#define STRLEN_PATH_TAGS         LIT_STRLEN(PATH_TAGS)\n #define PATH_REMOTES             \"remotes/\"\n-#define STRLEN_PATH_REMOTES      8\n+#define STRLEN_PATH_REMOTES      LIT_STRLEN(PATH_REMOTES)\n #define PATH_REFS_HEADS          PATH_REFS PATH_HEADS\n #define STRLEN_PATH_REFS_HEADS   (STRLEN_PATH_REFS+STRLEN_PATH_HEADS)\n #define PATH_REFS_TAGS           PATH_REFS PATH_TAGS\n-- \n1.5.3.rc5.11.g312e\n"},{"id":"54349","messageId":"7vd4w1l872.fsf@gitster.siamese.dyndns.org","threadId":"10075","inReplyTo":"200709291400.13880.andyparkins@gmail.com","subject":"Re: [PATCH 2/2] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-29T16:58:41Z","receivedAt":"2007-09-29T16:58:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andy Parkins <andyparkins@gmail.com> writes:\n\n> Bill Lear pointed out that the following:\n>\n>  #define PATH_REMOTES             \"remotes/\"\n>  #define STRLEN_PATH_REMOTES      8\n>\n> Could be replaced by the less error-prone\n>\n>  #define PATH_REMOTES \"remotes/\"\n>  #define LIT_STRLEN(S) ((sizeof(S) / sizeof(S[0])) -1)\n>  #define STRLEN_PATH_REMOTES LIT_STRLEN(PATH_REMOTES)\n>\n> which is what this patch does.\n\nI do not think the above is wrong per se, but doesn't a good\ncompiler optimize\n\n\t#define PATH_REMOTES\t\"remotes/\"\n\t#define STRLEN_PATH_REMOTES strlen(PATH_REMOTES)\n\n\t... and much later in some *.c file that includes the\n\t... above and <string.h>\n\n        foo = strlen(PATH_REMOTES)\n\nanyway?\n"},{"id":"54353","messageId":"alpine.LFD.0.999.0709291103380.3579@woody.linux-foundation.org","threadId":"10075","inReplyTo":"7vd4w1l872.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH 2/2] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-09-29T18:15:21Z","receivedAt":"2007-09-29T18:15:21Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Sat, 29 Sep 2007, Junio C Hamano wrote:\n> \n> I do not think the above is wrong per se, but doesn't a good\n> compiler optimize\n> \n> \t#define PATH_REMOTES\t\"remotes/\"\n> \t#define STRLEN_PATH_REMOTES strlen(PATH_REMOTES)\n\nGcc does, yes.\n\nHOWEVER.\n\nUsing \"strlen()\" may be optimized at compile-time, but it still ends up \nresulting in illegal C code if the constant needs to be a constant in the \nsemantic sense.\n\nIOW, the \"sizeof()\" trick can be portably used for things like array \ndeclarations etc. But strlen() cannot. Ie\n\n\tchar array[sizeof(\"hello\")-1];\n\nis legal in non-function scope, but doing the same with \"strlen()\" is not.\n\n(That said, gcc has been known to accept bad C code, and will in fact \naccept this one too!)\n\n\t\tLinus\n"}]}