From: René Scharfe Date: Mon, 27 Feb 2017 22:54:22 GMT Subject: Re: [PATCH 2/2] commit: don't check for space twice when looking for header Message-ID: In-Reply-To: <1dd0884c-032c-fb04-67f6-8b181fd65eea@gmail.com> Am 27.02.2017 um 23:27 schrieb Jakub Narębski: > W dniu 25.02.2017 o 20:27, René Scharfe pisze: >> Both standard_header_field() and excluded_header_field() check if >> there's a space after the buffer that's handed to them. We already >> check in the caller if that space is present. Don't bother calling >> the functions if it's missing, as they are guaranteed to return 0 in >> that case, and remove the now redundant checks from them. >> >> Signed-off-by: Rene Scharfe >> --- >> commit.c | 18 ++++++++---------- >> 1 file changed, 8 insertions(+), 10 deletions(-) >> >> diff --git a/commit.c b/commit.c >> index 173c6d3818..fab8269731 100644 >> --- a/commit.c >> +++ b/commit.c >> @@ -1308,11 +1308,11 @@ void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data) >> >> static inline int standard_header_field(const char *field, size_t len) >> { >> - return ((len == 4 && !memcmp(field, "tree ", 5)) || >> - (len == 6 && !memcmp(field, "parent ", 7)) || >> - (len == 6 && !memcmp(field, "author ", 7)) || >> - (len == 9 && !memcmp(field, "committer ", 10)) || >> - (len == 8 && !memcmp(field, "encoding ", 9))); >> + return ((len == 4 && !memcmp(field, "tree", 4)) || >> + (len == 6 && !memcmp(field, "parent", 6)) || >> + (len == 6 && !memcmp(field, "author", 6)) || >> + (len == 9 && !memcmp(field, "committer", 9)) || >> + (len == 8 && !memcmp(field, "encoding", 8))); > > I agree (for what it is worth from me) with the rest of changes, > but I think current code is better self-documenting for this > function. Having a function that is given a buffer/length pair and accessing the byte after it raises questions, though. :) Nicer than keeping the space would be to use excluded_header_field() for standard headers as well as a next step, I think -- but that would be a bit slower. > >> } >> >> static int excluded_header_field(const char *field, size_t len, const char **exclude) >> @@ -1322,8 +1322,7 @@ static int excluded_header_field(const char *field, size_t len, const char **exc >> >> while (*exclude) { >> size_t xlen = strlen(*exclude); >> - if (len == xlen && >> - !memcmp(field, *exclude, xlen) && field[xlen] == ' ') >> + if (len == xlen && !memcmp(field, *exclude, xlen)) >> return 1; >> exclude++; >> } >> @@ -1357,9 +1356,8 @@ static struct commit_extra_header *read_commit_extra_header_lines( >> eof = memchr(line, ' ', next - line); >> if (!eof) >> eof = next; >> - >> - if (standard_header_field(line, eof - line) || >> - excluded_header_field(line, eof - line, exclude)) >> + else if (standard_header_field(line, eof - line) || >> + excluded_header_field(line, eof - line, exclude)) >> continue; >> >> it = xcalloc(1, sizeof(*it)); >> >