{"thread":{"id":"36165","subject":"[PATCH] [GSoC] Use strchrnul to save additional scan of string","startedAt":"2014-03-15T00:49:08Z","lastAt":"2014-03-15T02:49:47Z","messageCount":2,"participants":["Shubham Chaudhary","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"236767","messageId":"CACqsmRbYwEKrF2+NbpLLnx2zrovwFV+OQe64Zpzg5WGpRqGfJw@mail.gmail.com","threadId":"36165","inReplyTo":null,"subject":"[PATCH] [GSoC] Use strchrnul to save additional scan of string","fromName":"Shubham Chaudhary","fromEmail":"shubham.chaudhary@kdemail.net","sentAt":"2014-03-15T00:49:08Z","receivedAt":"2014-03-15T00:49:08Z","isPatch":true,"sender":{"key":"shubham.chaudhary@kdemail.net","avatar":null},"body":">From c422507408824403ed18e89ec0bbc32b8764e09c Mon Sep 17 00:00:00 2001\nFrom: Shubham Chaudhary <shubham.chaudhary@kdemail.net>\nDate: Sat, 15 Mar 2014 05:56:18 +0530\nSubject: [PATCH] [GSoC] Use strchrnul to save additional scan of string\n\nSome strings are scanned twice unnecessarily, once with strchr() and\nthen with strlen().\nI rewrote these sites using strchrnul() when appropriate.\n\nSigned-off-by: Shubham Chaudhary <shubham.chaudhary@kdemail.net>\n---\n archive.c                              |  4 ++--\n builtin/blame.c                        |  7 ++-----\n builtin/fmt-merge-msg.c                |  8 ++++----\n builtin/mailinfo.c                     |  8 ++------\n builtin/reset.c                        |  4 ++--\n builtin/shortlog.c                     | 10 +++-------\n cache-tree.c                           |  9 +++------\n contrib/examples/builtin-fetch--tool.c |  3 +--\n daemon.c                               |  4 +---\n diff.c                                 |  7 ++-----\n fast-import.c                          | 17 ++++-------------\n match-trees.c                          |  9 +++------\n parse-options.c                        |  5 +----\n pretty.c                               |  5 ++---\n remote-testsvn.c                       |  4 ++--\n ws.c                                   |  7 ++-----\n 16 files changed, 36 insertions(+), 75 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 346f3b2..d196215 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -259,8 +259,8 @@ static void parse_treeish_arg(const char **argv,\n \t/* Remotes are only allowed to fetch actual refs */\n \tif (remote) {\n \t\tchar *ref = NULL;\n-\t\tconst char *colon = strchr(name, ':');\n-\t\tint refnamelen = colon ? colon - name : strlen(name);\n+\t\tconst char *colon = strchrnul(name, ':');\n+\t\tint refnamelen = colon - name;\n\n \t\tif (!dwim_ref(name, refnamelen, sha1, &ref))\n \t\t\tdie(\"no such ref: %.*s\", refnamelen, name);\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex e5b5d71..24c9676 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1332,11 +1332,8 @@ static void get_ac_line(const char *inbuf,\nconst char *what,\n \tif (!tmp)\n \t\tgoto error_out;\n \ttmp += strlen(what);\n-\tendp = strchr(tmp, '\\n');\n-\tif (!endp)\n-\t\tlen = strlen(tmp);\n-\telse\n-\t\tlen = endp - tmp;\n+\tendp = strchrnul(tmp, '\\n');\n+\tlen = endp - tmp;\n\n \tif (split_ident_line(&ident, tmp, len)) {\n \terror_out:\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 3906eda..9e8da5b 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -539,12 +539,12 @@ static void find_merge_parents(struct\nmerge_parents *result,\n \twhile (pos < in->len) {\n \t\tint len;\n \t\tchar *p = in->buf + pos;\n-\t\tchar *newline = strchr(p, '\\n');\n+\t\tchar *newline = strchrnul(p, '\\n');\n \t\tunsigned char sha1[20];\n \t\tstruct commit *parent;\n \t\tstruct object *obj;\n\n-\t\tlen = newline ? newline - p : strlen(p);\n+\t\tlen = newline - p;\n \t\tpos += len + !!newline;\n\n \t\tif (len < 43 ||\n@@ -615,8 +615,8 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,\n \t\tint len;\n \t\tchar *newline, *p = in->buf + pos;\n\n-\t\tnewline = strchr(p, '\\n');\n-\t\tlen = newline ? newline - p : strlen(p);\n+\t\tnewline = strchrnul(p, '\\n');\n+\t\tlen = newline - p;\n \t\tpos += len + !!newline;\n \t\ti++;\n \t\tp[len] = 0;\ndiff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\nindex 2c3cd8e..f901cf3 100644\n--- a/builtin/mailinfo.c\n+++ b/builtin/mailinfo.c\n@@ -902,12 +902,8 @@ static void output_header_lines(FILE *fout, const\nchar *hdr, const struct strbuf\n {\n \tconst char *sp = data->buf;\n \twhile (1) {\n-\t\tchar *ep = strchr(sp, '\\n');\n-\t\tint len;\n-\t\tif (!ep)\n-\t\t\tlen = strlen(sp);\n-\t\telse\n-\t\t\tlen = ep - sp;\n+\t\tchar *ep = strchrnul(sp, '\\n');\n+\t\tint len = ep - sp;\n \t\tfprintf(fout, \"%s: %.*s\\n\", hdr, len, sp);\n \t\tif (!ep)\n \t\t\tbreak;\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4fd1c6c..fa0ecf5 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -103,8 +103,8 @@ static void print_new_head_line(struct commit *commit)\n \t\tconst char *eol;\n \t\tsize_t len;\n \t\tbody += 2;\n-\t\teol = strchr(body, '\\n');\n-\t\tlen = eol ? eol - body : strlen(body);\n+\t\teol = strchrnul(body, '\\n');\n+\t\tlen = eol - body;\n \t\tprintf(\" %.*s\\n\", (int) len, body);\n \t}\n \telse\ndiff --git a/builtin/shortlog.c b/builtin/shortlog.c\nindex 4b7e536..f03174f 100644\n--- a/builtin/shortlog.c\n+++ b/builtin/shortlog.c\n@@ -62,9 +62,7 @@ static void insert_one_record(struct shortlog *log,\n \t/* Skip any leading whitespace, including any blank lines. */\n \twhile (*oneline && isspace(*oneline))\n \t\toneline++;\n-\teol = strchr(oneline, '\\n');\n-\tif (!eol)\n-\t\teol = oneline + strlen(oneline);\n+\teol = strchrnul(oneline, '\\n');\n \tif (starts_with(oneline, \"[PATCH\")) {\n \t\tchar *eob = strchr(oneline, ']');\n \t\tif (eob && (!eol || eob < eol))\n@@ -116,11 +114,9 @@ void shortlog_add_commit(struct shortlog *log,\nstruct commit *commit)\n \tpp_commit_easy(CMIT_FMT_RAW, commit, &buf);\n \tbuffer = buf.buf;\n \twhile (*buffer && *buffer != '\\n') {\n-\t\tconst char *eol = strchr(buffer, '\\n');\n+\t\tconst char *eol = strchrnul(buffer, '\\n');\n\n-\t\tif (eol == NULL)\n-\t\t\teol = buffer + strlen(buffer);\n-\t\telse\n+\t\tif (*eol)\n \t\t\teol++;\n\n \t\tif (starts_with(buffer, \"author \"))\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 0bbec43..a658e36 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -121,11 +121,11 @@ void cache_tree_invalidate_path(struct\ncache_tree *it, const char *path)\n\n \tif (!it)\n \t\treturn;\n-\tslash = strchr(path, '/');\n+\tslash = strchrnul(path, '/');\n+\tnamelen = slash - path;\n \tit->entry_count = -1;\n \tif (!slash) {\n \t\tint pos;\n-\t\tnamelen = strlen(path);\n \t\tpos = subtree_pos(it, path, namelen);\n \t\tif (0 <= pos) {\n \t\t\tcache_tree_free(&it->down[pos]->cache_tree);\n@@ -143,7 +143,6 @@ void cache_tree_invalidate_path(struct cache_tree\n*it, const char *path)\n \t\t}\n \t\treturn;\n \t}\n-\tnamelen = slash - path;\n \tdown = find_subtree(it, path, namelen, 0);\n \tif (down)\n \t\tcache_tree_invalidate_path(down->cache_tree, slash + 1);\n@@ -554,9 +553,7 @@ static struct cache_tree *cache_tree_find(struct\ncache_tree *it, const char *pat\n \t\tconst char *slash;\n \t\tstruct cache_tree_sub *sub;\n\n-\t\tslash = strchr(path, '/');\n-\t\tif (!slash)\n-\t\t\tslash = path + strlen(path);\n+\t\tslash = strchrnul(path, '/');\n \t\t/* between path and slash is the name of the\n \t\t * subtree to look for.\n \t\t */\ndiff --git a/contrib/examples/builtin-fetch--tool.c\nb/contrib/examples/builtin-fetch--tool.c\nindex 8bc8c75..1664a29 100644\n--- a/contrib/examples/builtin-fetch--tool.c\n+++ b/contrib/examples/builtin-fetch--tool.c\n@@ -382,8 +382,7 @@ static int expand_refs_wildcard(const char\n*ls_remote_result, int numrefs,\n\n \t\t\twhile (*ls && isspace(*ls))\n \t\t\t\tls++;\n-\t\t\tnext = strchr(ls, '\\n');\n-\t\t\teol = !next ? (ls + strlen(ls)) : next;\n+\t\t\teol = next = strchrnul(ls, '\\n');\n \t\t\tif (!memcmp(\"^{}\", eol-3, 3))\n \t\t\t\tcontinue;\n \t\t\tif (eol - ls < 40)\ndiff --git a/daemon.c b/daemon.c\nindex eba1255..f0100ad 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -131,9 +131,7 @@ static const char *path_ok(char *directory)\n \t\t\t * \"~alice/%s/foo\".\n \t\t\t */\n \t\t\tint namlen, restlen = strlen(dir);\n-\t\t\tchar *slash = strchr(dir, '/');\n-\t\t\tif (!slash)\n-\t\t\t\tslash = dir + restlen;\n+\t\t\tchar *slash = strchrnul(dir, '/');\n \t\t\tnamlen = slash - dir;\n \t\t\trestlen -= namlen;\n \t\t\tloginfo(\"userpath <%s>, request <%s>, namlen %d, restlen %d, slash\n<%s>\", user_path, dir, namlen, restlen, slash);\ndiff --git a/diff.c b/diff.c\nindex 7c59bfe..d3c3176 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3365,11 +3365,8 @@ static int opt_arg(const char *arg, int\narg_short, const char *arg_long, int *va\n \tif (c != '-')\n \t\treturn 0;\n \targ++;\n-\teq = strchr(arg, '=');\n-\tif (eq)\n-\t\tlen = eq - arg;\n-\telse\n-\t\tlen = strlen(arg);\n+\teq = strchrnul(arg, '=');\n+\tlen = eq - arg;\n \tif (!len || strncmp(arg, arg_long, len))\n \t\treturn 0;\n \tif (eq) {\ndiff --git a/fast-import.c b/fast-import.c\nindex 4fd18a3..fbdac17 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1485,11 +1485,8 @@ static int tree_content_set(\n \tunsigned int i, n;\n \tstruct tree_entry *e;\n\n-\tslash1 = strchr(p, '/');\n-\tif (slash1)\n-\t\tn = slash1 - p;\n-\telse\n-\t\tn = strlen(p);\n+\tslash1 = strchrnul(p, '/');\n+\tn = slash1 - p;\n \tif (!n)\n \t\tdie(\"Empty path component found in input\");\n \tif (!slash1 && !S_ISDIR(mode) && subtree)\n@@ -1577,10 +1574,7 @@ static int tree_content_remove(\n \tstruct tree_entry *e;\n\n \tslash1 = strchr(p, '/');\n-\tif (slash1)\n-\t\tn = slash1 - p;\n-\telse\n-\t\tn = strlen(p);\n+\tn = slash1 - p;\n\n \tif (!root->tree)\n \t\tload_tree(root);\n@@ -1645,10 +1639,7 @@ static int tree_content_get(\n \tstruct tree_entry *e;\n\n \tslash1 = strchr(p, '/');\n-\tif (slash1)\n-\t\tn = slash1 - p;\n-\telse\n-\t\tn = strlen(p);\n+\tn = slash1 - p;\n \tif (!n && !allow_root)\n \t\tdie(\"Empty path component found in input\");\n\ndiff --git a/match-trees.c b/match-trees.c\nindex 7873cde..4ac5c3e 100644\n--- a/match-trees.c\n+++ b/match-trees.c\n@@ -182,13 +182,10 @@ static int splice_tree(const unsigned char *hash1,\n \tenum object_type type;\n \tint status;\n\n-\tsubpath = strchr(prefix, '/');\n-\tif (!subpath)\n-\t\ttoplen = strlen(prefix);\n-\telse {\n-\t\ttoplen = subpath - prefix;\n+\tsubpath = strchrnul(prefix, '/');\n+\ttoplen = subpath - prefix;\n+\tif (subpath)\n \t\tsubpath++;\n-\t}\n\n \tbuf = read_sha1_file(hash1, &type, &sz);\n \tif (!buf)\ndiff --git a/parse-options.c b/parse-options.c\nindex 7b8d3fa..a5fa0b8 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -223,13 +223,10 @@ static int parse_long_opt(struct parse_opt_ctx_t\n*p, const char *arg,\n                           const struct option *options)\n {\n \tconst struct option *all_opts = options;\n-\tconst char *arg_end = strchr(arg, '=');\n+\tconst char *arg_end = strchrnul(arg, '=');\n \tconst struct option *abbrev_option = NULL, *ambiguous_option = NULL;\n \tint abbrev_flags = 0, ambiguous_flags = 0;\n\n-\tif (!arg_end)\n-\t\targ_end = arg + strlen(arg);\n-\n \tfor (; options->type != OPTION_END; options++) {\n \t\tconst char *rest, *long_name = options->long_name;\n \t\tint flags = 0, opt_flags = 0;\ndiff --git a/pretty.c b/pretty.c\nindex 87db08b..75aad57 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -549,14 +549,13 @@ static char *get_header(const struct commit\n*commit, const char *msg,\n \tconst char *line = msg;\n\n \twhile (line) {\n-\t\tconst char *eol = strchr(line, '\\n'), *next;\n+\t\tconst char *eol = strchrnul(line, '\\n'), *next;\n\n \t\tif (line == eol)\n \t\t\treturn NULL;\n-\t\tif (!eol) {\n+\t\tif (*eol != '\\n') {\n \t\t\twarning(\"malformed commit (header is missing newline): %s\",\n \t\t\t\tsha1_to_hex(commit->object.sha1));\n-\t\t\teol = line + strlen(line);\n \t\t\tnext = NULL;\n \t\t} else\n \t\t\tnext = eol + 1;\ndiff --git a/remote-testsvn.c b/remote-testsvn.c\nindex 078f1ff..6be55cb 100644\n--- a/remote-testsvn.c\n+++ b/remote-testsvn.c\n@@ -78,8 +78,8 @@ static int parse_rev_note(const char *msg, struct\nrev_note *res)\n \tsize_t len;\n\n \twhile (*msg) {\n-\t\tend = strchr(msg, '\\n');\n-\t\tlen = end ? end - msg : strlen(msg);\n+\t\tend = strchrnul(msg, '\\n');\n+\t\tlen = end - msg;\n\n \t\tkey = \"Revision-number: \";\n \t\tif (starts_with(msg, key)) {\ndiff --git a/ws.c b/ws.c\nindex b498d75..ea4b2b1 100644\n--- a/ws.c\n+++ b/ws.c\n@@ -33,11 +33,8 @@ unsigned parse_whitespace_rule(const char *string)\n \t\tint negated = 0;\n\n \t\tstring = string + strspn(string, \", \\t\\n\\r\");\n-\t\tep = strchr(string, ',');\n-\t\tif (!ep)\n-\t\t\tlen = strlen(string);\n-\t\telse\n-\t\t\tlen = ep - string;\n+\t\tep = strchrnul(string, ',');\n+\t\tlen = ep - string;\n\n \t\tif (*string == '-') {\n \t\t\tnegated = 1;\n-- \n1.8.3.2\n\n\n\n-- \nRegards,\nShubham Chaudhary (UTC +05:30)\nhttp://www.shubhamchaudhary.in\n"},{"id":"236775","messageId":"20140315024946.GA7031@sigill.intra.peff.net","threadId":"36165","inReplyTo":"CACqsmRbYwEKrF2+NbpLLnx2zrovwFV+OQe64Zpzg5WGpRqGfJw@mail.gmail.com","subject":"Re: [PATCH] [GSoC] Use strchrnul to save additional scan of string","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-03-15T02:49:47Z","receivedAt":"2014-03-15T02:49:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Mar 15, 2014 at 06:19:08AM +0530, Shubham Chaudhary wrote:\n\n> From c422507408824403ed18e89ec0bbc32b8764e09c Mon Sep 17 00:00:00 2001\n\nYou can drop this line; it's just part of the mbox format.\n\n> From: Shubham Chaudhary <shubham.chaudhary@kdemail.net>\n> Date: Sat, 15 Mar 2014 05:56:18 +0530\n> Subject: [PATCH] [GSoC] Use strchrnul to save additional scan of string\n\nAnd these are redundant with your mail headers. You can drop all of them.\n\nYour patch also appears to be whitespace-damaged (it looks like extra\nwrawpping). Consider using git-send-email, which takes care of all of\nthese issues.\n\n> diff --git a/archive.c b/archive.c\n> index 346f3b2..d196215 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -259,8 +259,8 @@ static void parse_treeish_arg(const char **argv,\n>  \t/* Remotes are only allowed to fetch actual refs */\n>  \tif (remote) {\n>  \t\tchar *ref = NULL;\n> -\t\tconst char *colon = strchr(name, ':');\n> -\t\tint refnamelen = colon ? colon - name : strlen(name);\n> +\t\tconst char *colon = strchrnul(name, ':');\n> +\t\tint refnamelen = colon - name;\n\nThis one is pretty straightforward, as we do not need ever look at\n\"colon\" after this. But this one:\n\n> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\n> index 2c3cd8e..f901cf3 100644\n> --- a/builtin/mailinfo.c\n> +++ b/builtin/mailinfo.c\n> @@ -902,12 +902,8 @@ static void output_header_lines(FILE *fout, const\n> char *hdr, const struct strbuf\n>  {\n>  \tconst char *sp = data->buf;\n>  \twhile (1) {\n> -\t\tchar *ep = strchr(sp, '\\n');\n> -\t\tint len;\n> -\t\tif (!ep)\n> -\t\t\tlen = strlen(sp);\n> -\t\telse\n> -\t\t\tlen = ep - sp;\n> +\t\tchar *ep = strchrnul(sp, '\\n');\n> +\t\tint len = ep - sp;\n>  \t\tfprintf(fout, \"%s: %.*s\\n\", hdr, len, sp);\n>  \t\tif (!ep)\n>  \t\t\tbreak;\n\n...does not look right. Before your patch, \"ep\" pointed to a newline, or\nNULL if we did not find one. After the fprintf, we try to break out of\nthe loop if we did not find a newline by checking \"!ep\". But that will\nnever trigger after your patch, and we loop forever reading bogus data\npast the end of the string.\n\nI didn't check the other sites; this might be the only problematic one,\nbut each one needs to be examined by hand. Please double-check the\nresult by running \"make test\", which does find this bug.\n\n-Peff\n"}]}