# [PATCH 2/2] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions 3 messages from 2007-09-29 to 2007-09-29. Participants: Andy Parkins, Junio C Hamano, Linus Torvalds. Thread: https://gitlist.dev/t/10075 ## Andy Parkins, 2007-09-29 13:00 Subject: [PATCH 2/2] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions Message-ID: <200709291400.13880.andyparkins@gmail.com> URL: https://gitlist.dev/e/200709291400.13880.andyparkins%40gmail.com ``` Bill Lear pointed out that the following: #define PATH_REMOTES "remotes/" #define STRLEN_PATH_REMOTES 8 Could be replaced by the less error-prone #define PATH_REMOTES "remotes/" #define LIT_STRLEN(S) ((sizeof(S) / sizeof(S[0])) -1) #define STRLEN_PATH_REMOTES LIT_STRLEN(PATH_REMOTES) which is what this patch does. Signed-off-by: Andy Parkins --- refs.h | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/refs.h b/refs.h index 1025d04..fb58889 100644 --- a/refs.h +++ b/refs.h @@ -13,16 +13,17 @@ struct ref_lock { #define REF_ISSYMREF 01 #define REF_ISPACKED 02 +#define LIT_STRLEN(S) ((sizeof(S) / sizeof(S[0])) -1) #define PATH_OBJECTS "objects/" -#define STRLEN_PATH_OBJECTS 8 +#define STRLEN_PATH_OBJECTS LIT_STRLEN(PATH_OBJECTS) #define PATH_REFS "refs/" -#define STRLEN_PATH_REFS 5 +#define STRLEN_PATH_REFS LIT_STRLEN(PATH_REFS) #define PATH_HEADS "heads/" -#define STRLEN_PATH_HEADS 6 +#define STRLEN_PATH_HEADS LIT_STRLEN(PATH_HEADS) #define PATH_TAGS "tags/" -#define STRLEN_PATH_TAGS 5 +#define STRLEN_PATH_TAGS LIT_STRLEN(PATH_TAGS) #define PATH_REMOTES "remotes/" -#define STRLEN_PATH_REMOTES 8 +#define STRLEN_PATH_REMOTES LIT_STRLEN(PATH_REMOTES) #define PATH_REFS_HEADS PATH_REFS PATH_HEADS #define STRLEN_PATH_REFS_HEADS (STRLEN_PATH_REFS+STRLEN_PATH_HEADS) #define PATH_REFS_TAGS PATH_REFS PATH_TAGS -- 1.5.3.rc5.11.g312e ``` ## Junio C Hamano, 2007-09-29 16:58 Subject: Re: [PATCH 2/2] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions Message-ID: <7vd4w1l872.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd4w1l872.fsf%40gitster.siamese.dyndns.org In-Reply-To: <200709291400.13880.andyparkins@gmail.com> ``` Andy Parkins writes: > Bill Lear pointed out that the following: > > #define PATH_REMOTES "remotes/" > #define STRLEN_PATH_REMOTES 8 > > Could be replaced by the less error-prone > > #define PATH_REMOTES "remotes/" > #define LIT_STRLEN(S) ((sizeof(S) / sizeof(S[0])) -1) > #define STRLEN_PATH_REMOTES LIT_STRLEN(PATH_REMOTES) > > which is what this patch does. I do not think the above is wrong per se, but doesn't a good compiler optimize #define PATH_REMOTES "remotes/" #define STRLEN_PATH_REMOTES strlen(PATH_REMOTES) ... and much later in some *.c file that includes the ... above and foo = strlen(PATH_REMOTES) anyway? ``` ## Linus Torvalds, 2007-09-29 18:15 Subject: Re: [PATCH 2/2] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions Message-ID: URL: https://gitlist.dev/e/alpine.LFD.0.999.0709291103380.3579%40woody.linux-foundation.org In-Reply-To: <7vd4w1l872.fsf@gitster.siamese.dyndns.org> ``` On Sat, 29 Sep 2007, Junio C Hamano wrote: > > I do not think the above is wrong per se, but doesn't a good > compiler optimize > > #define PATH_REMOTES "remotes/" > #define STRLEN_PATH_REMOTES strlen(PATH_REMOTES) Gcc does, yes. HOWEVER. Using "strlen()" may be optimized at compile-time, but it still ends up resulting in illegal C code if the constant needs to be a constant in the semantic sense. IOW, the "sizeof()" trick can be portably used for things like array declarations etc. But strlen() cannot. Ie char array[sizeof("hello")-1]; is legal in non-function scope, but doing the same with "strlen()" is not. (That said, gcc has been known to accept bad C code, and will in fact accept this one too!) Linus ```