# [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop 5 messages from 2014-02-27 to 2014-02-28. Participants: Sun He, Junio C Hamano, David Kastrup, Jeff King. Thread: https://gitlist.dev/t/35972 ## Sun He, 2014-02-27 12:13 Subject: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop Message-ID: <1393503197-29669-1-git-send-email-sunheehnus@gmail.com> URL: https://gitlist.dev/e/1393503197-29669-1-git-send-email-sunheehnus%40gmail.com ``` Signed-off-by: Sun He --- git-compat-util.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/git-compat-util.h b/git-compat-util.h index cbd86c3..4daa6cf 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix); static inline const char *skip_prefix(const char *str, const char *prefix) { - size_t len = strlen(prefix); - return strncmp(str, prefix, len) ? NULL : str + len; + while( *prefix != '\0' && *str++ == *prefix++ ); + return *prefix == '\0' ? str : NULL; } #if defined(NO_MMAP) || defined(USE_WIN32_MMAP) -- 1.7.1 ``` ## Junio C Hamano, 2014-02-27 19:35 Subject: Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop Message-ID: URL: https://gitlist.dev/e/xmqqd2i8z6um.fsf%40gitster.dls.corp.google.com In-Reply-To: <1393503197-29669-1-git-send-email-sunheehnus@gmail.com> ``` Sun He writes: > Signed-off-by: Sun He > --- > git-compat-util.h | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/git-compat-util.h b/git-compat-util.h > index cbd86c3..4daa6cf 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix); > > static inline const char *skip_prefix(const char *str, const char *prefix) > { > - size_t len = strlen(prefix); > - return strncmp(str, prefix, len) ? NULL : str + len; > + while( *prefix != '\0' && *str++ == *prefix++ ); > + return *prefix == '\0' ? str : NULL; Documentation/CodingGuidelines? > } > > #if defined(NO_MMAP) || defined(USE_WIN32_MMAP) ``` ## David Kastrup, 2014-02-27 20:33 Subject: Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop Message-ID: <87k3cg47o6.fsf@fencepost.gnu.org> URL: https://gitlist.dev/e/87k3cg47o6.fsf%40fencepost.gnu.org In-Reply-To: ``` Junio C Hamano writes: > Sun He writes: > >> Signed-off-by: Sun He >> --- >> git-compat-util.h | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/git-compat-util.h b/git-compat-util.h >> index cbd86c3..4daa6cf 100644 >> --- a/git-compat-util.h >> +++ b/git-compat-util.h >> @@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix); >> >> static inline const char *skip_prefix(const char *str, const char *prefix) >> { >> - size_t len = strlen(prefix); >> - return strncmp(str, prefix, len) ? NULL : str + len; >> + while( *prefix != '\0' && *str++ == *prefix++ ); >> + return *prefix == '\0' ? str : NULL; > > Documentation/CodingGuidelines? Mostly relevant for tabification here, not helping much otherwise. In particular, does not contain the advice "empty statements should appear on a line of their own" which would help with readability here. -- David Kastrup ``` ## Junio C Hamano, 2014-02-27 22:16 Subject: Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop Message-ID: URL: https://gitlist.dev/e/xmqq38j4xkuq.fsf%40gitster.dls.corp.google.com In-Reply-To: <87k3cg47o6.fsf@fencepost.gnu.org> ``` David Kastrup writes: > Junio C Hamano writes: > >> Sun He writes: >> >>> Signed-off-by: Sun He >>> --- >>> git-compat-util.h | 4 ++-- >>> 1 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/git-compat-util.h b/git-compat-util.h >>> index cbd86c3..4daa6cf 100644 >>> --- a/git-compat-util.h >>> +++ b/git-compat-util.h >>> @@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix); >>> >>> static inline const char *skip_prefix(const char *str, const char *prefix) >>> { >>> - size_t len = strlen(prefix); >>> - return strncmp(str, prefix, len) ? NULL : str + len; >>> + while( *prefix != '\0' && *str++ == *prefix++ ); >>> + return *prefix == '\0' ? str : NULL; >> >> Documentation/CodingGuidelines? > > Mostly relevant for tabification here, not helping much otherwise. "Imitate existing code" would let you spot that we have SP outside the () pair, not inside, for controls like while/for/if, and we usually do not explicitly compare things with 0, NULL or '\0'. Together with the "empty statement should occupy its own line" you mentioned, I tend to agree with you that some people may benefit from them explicitly spelled out. ``` ## Jeff King, 2014-02-28 06:17 Subject: Re: [PATCH] Rewrite git-compat-util.h:skip_prefix() as a loop Message-ID: <20140228061725.GD32556@sigill.intra.peff.net> URL: https://gitlist.dev/e/20140228061725.GD32556%40sigill.intra.peff.net In-Reply-To: <87k3cg47o6.fsf@fencepost.gnu.org> ``` On Thu, Feb 27, 2014 at 09:33:45PM +0100, David Kastrup wrote: > >> diff --git a/git-compat-util.h b/git-compat-util.h > >> index cbd86c3..4daa6cf 100644 > >> --- a/git-compat-util.h > >> +++ b/git-compat-util.h > >> @@ -357,8 +357,8 @@ extern int suffixcmp(const char *str, const char *suffix); > >> > >> static inline const char *skip_prefix(const char *str, const char *prefix) > >> { > >> - size_t len = strlen(prefix); > >> - return strncmp(str, prefix, len) ? NULL : str + len; > >> + while( *prefix != '\0' && *str++ == *prefix++ ); > >> + return *prefix == '\0' ? str : NULL; > > > > Documentation/CodingGuidelines? > > Mostly relevant for tabification here, not helping much otherwise. In > particular, does not contain the advice "empty statements should appear > on a line of their own" which would help with readability here. Also whitespace in the "while", which I could not find mentioned in CodingGuidelines either. Maybe: -- >8 -- Subject: [PATCH] CodingGuidelines: mention C whitespace rules We are fairly consistent about these, so most are covered by "follow existing style", but it doesn't hurt to be explicit. Signed-off-by: Jeff King --- Documentation/CodingGuidelines | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines index ef67b53..ed432a8 100644 --- a/Documentation/CodingGuidelines +++ b/Documentation/CodingGuidelines @@ -126,6 +126,17 @@ For C programs: "char * string". This makes it easier to understand code like "char *string, c;". + - Use whitespace around operators and keywords, but not inside + parentheses and not around functions. So: + + while (condition) + func(bar + 1); + + and not: + + while( condition ) + func (bar+1); + - We avoid using braces unnecessarily. I.e. if (bla) { -- 1.8.5.2.500.g8060133 ```