{"thread":{"id":"35546","subject":"[PATCH 00/12] Hard coded string length cleanup","startedAt":"2013-12-18T14:53:45Z","lastAt":"2013-12-28T09:54:40Z","messageCount":31,"participants":["Nguyễn Thái Ngọc Duy","Kent R. Spillner","Junio C Hamano","René Scharfe","Duy Nguyen","Johannes Sixt","Jeff King","Christian Couder"],"isPatch":true,"patchVersion":1,"patchTotal":12},"messages":[{"id":"232152","messageId":"1387378437-20646-1-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":null,"subject":"[PATCH 00/12] Hard coded string length cleanup","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:45Z","receivedAt":"2013-12-18T14:53:45Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"I reimplemented skip_prefix() again just to realize this function\nalready exists. Which reminds me there are a bunch of places that\ncould benefit from this function, the same reason that I wanted to\nreimplement it.\n\nSo this is series to make it more popular (so hopefully I'll see it\nused somewhere and know that it exists) and the code cleaner. The\npattern \"compare a string, then skip the compared part by a hard coded\nstring length\" is almost killed. I left a few in places for those who\nwant to contribute :)\n\nNguyễn Thái Ngọc Duy (12):\n Make starts_with() a wrapper of skip_prefix()\n Convert starts_with() to skip_prefix() for option parsing\n Add and use skip_prefix_defval()\n Replace some use of starts_with() with skip_prefix()\n Convert a lot of starts_with() to skip_prefix()\n fetch.c: replace some use of starts_with() with skip_prefix()\n connect.c: replace some use of starts_with() with skip_prefix()\n refs.c: replace some use of starts_with() with skip_prefix()\n diff.c: reduce code duplication in --stat-xxx parsing\n environment.c: replace starts_with() in strip_namespace() with skip_prefix()\n diff.c: convert diff_scoreopt_parse to use skip_prefix()\n refs.c: use skip_prefix() in prune_ref()\n\n builtin/branch.c | 3 +-\n builtin/checkout.c | 6 +-\n builtin/fast-export.c | 3 +-\n builtin/fetch-pack.c | 13 ++--\n builtin/fetch.c | 16 ++---\n builtin/for-each-ref.c | 9 +--\n builtin/index-pack.c | 17 +++---\n builtin/ls-remote.c | 9 +--\n builtin/mailinfo.c | 11 ++--\n builtin/merge.c | 12 ++--\n builtin/reflog.c | 9 +--\n builtin/remote.c | 3 +-\n builtin/rev-parse.c | 41 ++++++-------\n builtin/send-pack.c | 18 +++---\n builtin/show-branch.c | 14 ++---\n builtin/unpack-objects.c | 5 +-\n builtin/update-ref.c | 21 +++----\n commit.c | 5 +-\n connect.c | 6 +-\n daemon.c | 75 +++++++++++------------\n diff.c | 153 +++++++++++++++++++++--------------------------\n environment.c | 4 +-\n fetch-pack.c | 9 +--\n git-compat-util.h | 15 ++++-\n git.c | 16 ++---\n http-backend.c | 5 +-\n http-push.c | 6 +-\n http.c | 5 +-\n log-tree.c | 5 +-\n merge-recursive.c | 13 ++--\n notes.c | 6 +-\n pager.c | 2 +-\n pathspec.c | 5 +-\n pretty.c | 3 +-\n refs.c | 20 ++++---\n revision.c | 60 +++++++++----------\n setup.c | 3 +-\n sha1_name.c | 12 +---\n strbuf.c | 9 ---\n tag.c | 7 +--\n transport-helper.c | 15 +++--\n transport.c | 14 +++--\n upload-pack.c | 5 +-\n wt-status.c | 15 ++---\n 44 files changed, 334 insertions(+), 369 deletions(-)\n\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232153","messageId":"1387378437-20646-2-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 01/12] Make starts_with() a wrapper of skip_prefix()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:46Z","receivedAt":"2013-12-18T14:53:46Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"starts_with() started out as a copy of prefixcmp(). But if we don't\ncare about the sorting order, the logic looks closer to\nskip_prefix(). This looks like a good thing to do.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n git-compat-util.h | 6 +++++-\n strbuf.c | 9 ---------\n 2 files changed, 5 insertions(+), 10 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex b73916b..84f1078 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -350,7 +350,6 @@ extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_lis\n extern void set_error_routine(void (*routine)(const char *err, va_list params));\n extern void set_die_is_recursing_routine(int (*routine)(void));\n \n-extern int starts_with(const char *str, const char *prefix);\n extern int prefixcmp(const char *str, const char *prefix);\n extern int ends_with(const char *str, const char *suffix);\n extern int suffixcmp(const char *str, const char *suffix);\n@@ -361,6 +360,11 @@ static inline const char *skip_prefix(const char *str, const char *prefix)\n \treturn strncmp(str, prefix, len) ? NULL : str + len;\n }\n \n+static inline int starts_with(const char *str, const char *prefix)\n+{\n+\treturn skip_prefix(str, prefix) != NULL;\n+}\n+\n #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n \n #ifndef PROT_READ\ndiff --git a/strbuf.c b/strbuf.c\nindex 83caf4a..bd4c0d8 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1,15 +1,6 @@\n #include \"cache.h\"\n #include \"refs.h\"\n \n-int starts_with(const char *str, const char *prefix)\n-{\n-\tfor (; ; str++, prefix++)\n-\t\tif (!*prefix)\n-\t\t\treturn 1;\n-\t\telse if (*str != *prefix)\n-\t\t\treturn 0;\n-}\n-\n int prefixcmp(const char *str, const char *prefix)\n {\n \tfor (; ; str++, prefix++)\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232154","messageId":"1387378437-20646-3-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:47Z","receivedAt":"2013-12-18T14:53:47Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The code that's not converted to use parse_options() often does\n\n if (!starts_with(arg, \"foo=\")) {\n value = atoi(arg + 4);\n }\n\nThis patch removes those magic numbers with skip_prefix()\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/fetch-pack.c | 13 +++++----\n builtin/index-pack.c | 17 +++++------\n builtin/ls-remote.c | 9 +++---\n builtin/mailinfo.c | 5 ++--\n builtin/reflog.c | 9 +++---\n builtin/rev-parse.c | 41 +++++++++++++-------------\n builtin/send-pack.c | 18 ++++++------\n builtin/unpack-objects.c | 5 ++--\n builtin/update-ref.c | 21 +++++++-------\n daemon.c | 75 ++++++++++++++++++++++++------------------------\n diff.c | 49 +++++++++++++++----------------\n git.c | 13 +++++----\n merge-recursive.c | 13 +++++----\n revision.c | 60 +++++++++++++++++++-------------------\n upload-pack.c | 5 ++--\n 15 files changed, 182 insertions(+), 171 deletions(-)\n\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 8b8978a2..2df1423 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -47,13 +47,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 1; i < argc && *argv[i] == '-'; i++) {\n \t\tconst char *arg = argv[i];\n+\t\tconst char *optarg;\n \n-\t\tif (starts_with(arg, \"--upload-pack=\")) {\n-\t\t\targs.uploadpack = arg + 14;\n+\t\tif ((optarg = skip_prefix(arg, \"--upload-pack=\")) != NULL) {\n+\t\t\targs.uploadpack = optarg;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--exec=\")) {\n-\t\t\targs.uploadpack = arg + 7;\n+\t\tif ((optarg = skip_prefix(arg, \"--exec=\")) != NULL) {\n+\t\t\targs.uploadpack = optarg;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(\"--quiet\", arg) || !strcmp(\"-q\", arg)) {\n@@ -89,8 +90,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\t\targs.verbose = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--depth=\")) {\n-\t\t\targs.depth = strtol(arg + 8, NULL, 0);\n+\t\tif ((optarg = skip_prefix(arg, \"--depth=\")) != NULL) {\n+\t\t\targs.depth = strtol(optarg, NULL, 0);\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(\"--no-progress\", arg)) {\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex 2f37a38..67eff7a 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -1511,6 +1511,7 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n+\t\tconst char *optarg;\n \n \t\tif (*arg == '-') {\n \t\t\tif (!strcmp(arg, \"--stdin\")) {\n@@ -1534,11 +1535,11 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tstat_only = 1;\n \t\t\t} else if (!strcmp(arg, \"--keep\")) {\n \t\t\t\tkeep_msg = \"\";\n-\t\t\t} else if (starts_with(arg, \"--keep=\")) {\n-\t\t\t\tkeep_msg = arg + 7;\n-\t\t\t} else if (starts_with(arg, \"--threads=\")) {\n+\t\t\t} else if ((optarg = skip_prefix(arg, \"--keep=\")) != NULL) {\n+\t\t\t\tkeep_msg = optarg;\n+\t\t\t} else if ((optarg = skip_prefix(arg, \"--threads=\")) != NULL) {\n \t\t\t\tchar *end;\n-\t\t\t\tnr_threads = strtoul(arg+10, &end, 0);\n+\t\t\t\tnr_threads = strtoul(optarg, &end, 0);\n \t\t\t\tif (!arg[10] || *end || nr_threads < 0)\n \t\t\t\t\tusage(index_pack_usage);\n #ifdef NO_PTHREADS\n@@ -1547,13 +1548,13 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\t \"ignoring %s\"), arg);\n \t\t\t\tnr_threads = 1;\n #endif\n-\t\t\t} else if (starts_with(arg, \"--pack_header=\")) {\n+\t\t\t} else if ((optarg = skip_prefix(arg, \"--pack_header=\")) != NULL) {\n \t\t\t\tstruct pack_header *hdr;\n \t\t\t\tchar *c;\n \n \t\t\t\thdr = (struct pack_header *)input_buffer;\n \t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n-\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n+\t\t\t\thdr->hdr_version = htonl(strtoul(optarg, &c, 10));\n \t\t\t\tif (*c != ',')\n \t\t\t\t\tdie(_(\"bad %s\"), arg);\n \t\t\t\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n@@ -1566,9 +1567,9 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\tif (index_name || (i+1) >= argc)\n \t\t\t\t\tusage(index_pack_usage);\n \t\t\t\tindex_name = argv[++i];\n-\t\t\t} else if (starts_with(arg, \"--index-version=\")) {\n+\t\t\t} else if ((optarg = skip_prefix(arg, \"--index-version=\")) != NULL) {\n \t\t\t\tchar *c;\n-\t\t\t\topts.version = strtoul(arg + 16, &c, 10);\n+\t\t\t\topts.version = strtoul(optarg, &c, 10);\n \t\t\t\tif (opts.version > 2)\n \t\t\t\t\tdie(_(\"bad %s\"), arg);\n \t\t\t\tif (*c == ',')\ndiff --git a/builtin/ls-remote.c b/builtin/ls-remote.c\nindex 39e5144..15c9fb3 100644\n--- a/builtin/ls-remote.c\n+++ b/builtin/ls-remote.c\n@@ -48,14 +48,15 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n+\t\tconst char *optarg;\n \n \t\tif (*arg == '-') {\n-\t\t\tif (starts_with(arg, \"--upload-pack=\")) {\n-\t\t\t\tuploadpack = arg + 14;\n+\t\t\tif ((optarg = skip_prefix(arg, \"--upload-pack=\")) != NULL) {\n+\t\t\t\tuploadpack = optarg;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--exec=\")) {\n-\t\t\t\tuploadpack = arg + 7;\n+\t\t\tif ((optarg = skip_prefix(arg, \"--exec=\")) != NULL) {\n+\t\t\t\tuploadpack = optarg;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(\"--tags\", arg) || !strcmp(\"-t\", arg)) {\ndiff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\nindex 2c3cd8e..2100e23 100644\n--- a/builtin/mailinfo.c\n+++ b/builtin/mailinfo.c\n@@ -1002,6 +1002,7 @@ static const char mailinfo_usage[] =\n int cmd_mailinfo(int argc, const char **argv, const char *prefix)\n {\n \tconst char *def_charset;\n+\tconst char *optarg;\n \n \t/* NEEDSWORK: might want to do the optional .git/ directory\n \t * discovery\n@@ -1020,8 +1021,8 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)\n \t\t\tmetainfo_charset = def_charset;\n \t\telse if (!strcmp(argv[1], \"-n\"))\n \t\t\tmetainfo_charset = NULL;\n-\t\telse if (starts_with(argv[1], \"--encoding=\"))\n-\t\t\tmetainfo_charset = argv[1] + 11;\n+\t\telse if ((optarg = skip_prefix(argv[1], \"--encoding=\")) != NULL)\n+\t\t\tmetainfo_charset = optarg;\n \t\telse if (!strcmp(argv[1], \"--scissors\"))\n \t\t\tuse_scissors = 1;\n \t\telse if (!strcmp(argv[1], \"--no-scissors\"))\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 852cff6..84a8bd9 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -608,15 +608,16 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n+\t\tconst char *optarg;\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n \t\t\tcb.dry_run = 1;\n-\t\telse if (starts_with(arg, \"--expire=\")) {\n-\t\t\tif (parse_expiry_date(arg + 9, &cb.expire_total))\n+\t\telse if ((optarg = skip_prefix(arg, \"--expire=\")) != NULL) {\n+\t\t\tif (parse_expiry_date(optarg, &cb.expire_total))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n \t\t\texplicit_expiry |= EXPIRE_TOTAL;\n \t\t}\n-\t\telse if (starts_with(arg, \"--expire-unreachable=\")) {\n-\t\t\tif (parse_expiry_date(arg + 21, &cb.expire_unreachable))\n+\t\telse if ((optarg = skip_prefix(arg, \"--expire-unreachable=\")) != NULL) {\n+\t\t\tif (parse_expiry_date(optarg, &cb.expire_unreachable))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n \t\t\texplicit_expiry |= EXPIRE_UNREACH;\n \t\t}\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 6e802fd..1a0bd12 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -505,6 +505,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \tgit_config(git_default_config, NULL);\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n+\t\tconst char *optarg;\n \n \t\tif (as_is) {\n \t\t\tif (show_file(arg, output_prefix) && as_is < 2)\n@@ -618,8 +619,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tfor_each_ref(show_reference, NULL);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--disambiguate=\")) {\n-\t\t\t\tfor_each_abbrev(arg + 15, show_abbrev, NULL);\n+\t\t\tif ((optarg = skip_prefix(arg, \"--disambiguate=\")) != NULL) {\n+\t\t\t\tfor_each_abbrev(optarg, show_abbrev, NULL);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--bisect\")) {\n@@ -627,8 +628,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tfor_each_ref_in(\"refs/bisect/good\", anti_reference, NULL);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--branches=\")) {\n-\t\t\t\tfor_each_glob_ref_in(show_reference, arg + 11,\n+\t\t\tif ((optarg = skip_prefix(arg, \"--branches=\")) != NULL) {\n+\t\t\t\tfor_each_glob_ref_in(show_reference, optarg,\n \t\t\t\t\t\"refs/heads/\", NULL);\n \t\t\t\tclear_ref_exclusion(&ref_excludes);\n \t\t\t\tcontinue;\n@@ -638,8 +639,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tclear_ref_exclusion(&ref_excludes);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--tags=\")) {\n-\t\t\t\tfor_each_glob_ref_in(show_reference, arg + 7,\n+\t\t\tif ((optarg = skip_prefix(arg, \"--tags=\")) != NULL) {\n+\t\t\t\tfor_each_glob_ref_in(show_reference, optarg,\n \t\t\t\t\t\"refs/tags/\", NULL);\n \t\t\t\tclear_ref_exclusion(&ref_excludes);\n \t\t\t\tcontinue;\n@@ -649,13 +650,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tclear_ref_exclusion(&ref_excludes);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--glob=\")) {\n-\t\t\t\tfor_each_glob_ref(show_reference, arg + 7, NULL);\n+\t\t\tif ((optarg = skip_prefix(arg, \"--glob=\")) != NULL) {\n+\t\t\t\tfor_each_glob_ref(show_reference, optarg, NULL);\n \t\t\t\tclear_ref_exclusion(&ref_excludes);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--remotes=\")) {\n-\t\t\t\tfor_each_glob_ref_in(show_reference, arg + 10,\n+\t\t\tif ((optarg = skip_prefix(arg, \"--remotes=\")) != NULL) {\n+\t\t\t\tfor_each_glob_ref_in(show_reference, optarg,\n \t\t\t\t\t\"refs/remotes/\", NULL);\n \t\t\t\tclear_ref_exclusion(&ref_excludes);\n \t\t\t\tcontinue;\n@@ -665,8 +666,8 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tclear_ref_exclusion(&ref_excludes);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--exclude=\")) {\n-\t\t\t\tadd_ref_exclusion(&ref_excludes, arg + 10);\n+\t\t\tif ((optarg = skip_prefix(arg, \"--exclude=\")) != NULL) {\n+\t\t\t\tadd_ref_exclusion(&ref_excludes, optarg);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--local-env-vars\")) {\n@@ -747,20 +748,20 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\t: \"false\");\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--since=\")) {\n-\t\t\t\tshow_datestring(\"--max-age=\", arg+8);\n+\t\t\tif ((optarg = skip_prefix(arg, \"--since=\")) != NULL) {\n+\t\t\t\tshow_datestring(\"--max-age=\", optarg);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--after=\")) {\n-\t\t\t\tshow_datestring(\"--max-age=\", arg+8);\n+\t\t\tif ((optarg = skip_prefix(arg, \"--after=\")) != NULL) {\n+\t\t\t\tshow_datestring(\"--max-age=\", optarg);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--before=\")) {\n-\t\t\t\tshow_datestring(\"--min-age=\", arg+9);\n+\t\t\tif ((optarg = skip_prefix(arg, \"--before=\")) != NULL) {\n+\t\t\t\tshow_datestring(\"--min-age=\", optarg);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--until=\")) {\n-\t\t\t\tshow_datestring(\"--min-age=\", arg+8);\n+\t\t\tif ((optarg = skip_prefix(arg, \"--until=\")) != NULL) {\n+\t\t\t\tshow_datestring(\"--min-age=\", optarg);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (show_flag(arg) && verify)\ndiff --git a/builtin/send-pack.c b/builtin/send-pack.c\nindex e7f0b97..9efc422 100644\n--- a/builtin/send-pack.c\n+++ b/builtin/send-pack.c\n@@ -113,18 +113,19 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \targv++;\n \tfor (i = 1; i < argc; i++, argv++) {\n \t\tconst char *arg = *argv;\n+\t\tconst char *optarg;\n \n \t\tif (*arg == '-') {\n-\t\t\tif (starts_with(arg, \"--receive-pack=\")) {\n-\t\t\t\treceivepack = arg + 15;\n+\t\t\tif ((optarg = skip_prefix(arg, \"--receive-pack=\")) != NULL) {\n+\t\t\t\treceivepack = optarg;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--exec=\")) {\n-\t\t\t\treceivepack = arg + 7;\n+\t\t\tif ((optarg = skip_prefix(arg, \"--exec=\")) != NULL) {\n+\t\t\t\treceivepack = optarg;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--remote=\")) {\n-\t\t\t\tremote_name = arg + 9;\n+\t\t\tif ((optarg = skip_prefix(arg, \"--remote=\")) != NULL) {\n+\t\t\t\tremote_name = optarg;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--all\")) {\n@@ -181,9 +182,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)\n \t\t\t\t\texit(1);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--\" CAS_OPT_NAME \"=\")) {\n-\t\t\t\tif (parse_push_cas_option(&cas,\n-\t\t\t\t\t\t\t strchr(arg, '=') + 1, 0) < 0)\n+\t\t\tif ((optarg = skip_prefix(arg, \"--\" CAS_OPT_NAME \"=\")) != NULL) {\n+\t\t\t\tif (parse_push_cas_option(&cas, optarg, 0) < 0)\n \t\t\t\t\texit(1);\n \t\t\t\tcontinue;\n \t\t\t}\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 62ff673..a7cd823 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -505,6 +505,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix)\n \n \tfor (i = 1 ; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n+\t\tconst char *optarg;\n \n \t\tif (*arg == '-') {\n \t\t\tif (!strcmp(arg, \"-n\")) {\n@@ -523,13 +524,13 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix)\n \t\t\t\tstrict = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(arg, \"--pack_header=\")) {\n+\t\t\tif ((optarg = skip_prefix(arg, \"--pack_header=\")) != NULL) {\n \t\t\t\tstruct pack_header *hdr;\n \t\t\t\tchar *c;\n \n \t\t\t\thdr = (struct pack_header *)buffer;\n \t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n-\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n+\t\t\t\thdr->hdr_version = htonl(strtoul(optarg, &c, 10));\n \t\t\t\tif (*c != ',')\n \t\t\t\t\tdie(\"bad %s\", arg);\n \t\t\t\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\ndiff --git a/builtin/update-ref.c b/builtin/update-ref.c\nindex 1292cfe..09c921a 100644\n--- a/builtin/update-ref.c\n+++ b/builtin/update-ref.c\n@@ -222,6 +222,7 @@ static void parse_cmd_option(const char *next)\n static void update_refs_stdin(void)\n {\n \tstruct strbuf cmd = STRBUF_INIT;\n+\tconst char *optarg;\n \n \t/* Read each line dispatch its command */\n \twhile (strbuf_getline(&cmd, stdin, line_termination) != EOF)\n@@ -229,16 +230,16 @@ static void update_refs_stdin(void)\n \t\t\tdie(\"empty command in input\");\n \t\telse if (isspace(*cmd.buf))\n \t\t\tdie(\"whitespace before command: %s\", cmd.buf);\n-\t\telse if (starts_with(cmd.buf, \"update \"))\n-\t\t\tparse_cmd_update(cmd.buf + 7);\n-\t\telse if (starts_with(cmd.buf, \"create \"))\n-\t\t\tparse_cmd_create(cmd.buf + 7);\n-\t\telse if (starts_with(cmd.buf, \"delete \"))\n-\t\t\tparse_cmd_delete(cmd.buf + 7);\n-\t\telse if (starts_with(cmd.buf, \"verify \"))\n-\t\t\tparse_cmd_verify(cmd.buf + 7);\n-\t\telse if (starts_with(cmd.buf, \"option \"))\n-\t\t\tparse_cmd_option(cmd.buf + 7);\n+\t\telse if ((optarg = skip_prefix(cmd.buf, \"update \")) != NULL)\n+\t\t\tparse_cmd_update(optarg);\n+\t\telse if ((optarg = skip_prefix(cmd.buf, \"create \")) != NULL)\n+\t\t\tparse_cmd_create(optarg);\n+\t\telse if ((optarg = skip_prefix(cmd.buf, \"delete \")) != NULL)\n+\t\t\tparse_cmd_delete(optarg);\n+\t\telse if ((optarg = skip_prefix(cmd.buf, \"verify \")) != NULL)\n+\t\t\tparse_cmd_verify(optarg);\n+\t\telse if ((optarg = skip_prefix(cmd.buf, \"option \")) != NULL)\n+\t\t\tparse_cmd_option(optarg);\n \t\telse\n \t\t\tdie(\"unknown command: %s\", cmd.buf);\n \ndiff --git a/daemon.c b/daemon.c\nindex 7bee953..9d3cc18 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -39,8 +39,8 @@ static int strict_paths;\n static int export_all_trees;\n \n /* Take all paths relative to this one if non-NULL */\n-static char *base_path;\n-static char *interpolated_path;\n+static const char *base_path;\n+static const char *interpolated_path;\n static int base_path_relaxed;\n \n /* Flag indicating client sent extra args. */\n@@ -253,7 +253,7 @@ static int daemon_error(const char *dir, const char *msg)\n \treturn -1;\n }\n \n-static char *access_hook;\n+static const char *access_hook;\n \n static int run_access_hook(struct daemon_service *service, const char *dir, const char *path)\n {\n@@ -1164,15 +1164,16 @@ int main(int argc, char **argv)\n \n \tfor (i = 1; i < argc; i++) {\n \t\tchar *arg = argv[i];\n+\t\tconst char *optarg;\n \n-\t\tif (starts_with(arg, \"--listen=\")) {\n-\t\t\tstring_list_append(&listen_addr, xstrdup_tolower(arg + 9));\n+\t\tif ((optarg = skip_prefix(arg, \"--listen=\")) != NULL) {\n+\t\t\tstring_list_append(&listen_addr, xstrdup_tolower(optarg));\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--port=\")) {\n+\t\tif ((optarg = skip_prefix(arg, \"--port=\")) != NULL) {\n \t\t\tchar *end;\n \t\t\tunsigned long n;\n-\t\t\tn = strtoul(arg+7, &end, 0);\n+\t\t\tn = strtoul(optarg, &end, 0);\n \t\t\tif (arg[7] && !*end) {\n \t\t\t\tlisten_port = n;\n \t\t\t\tcontinue;\n@@ -1199,20 +1200,20 @@ int main(int argc, char **argv)\n \t\t\texport_all_trees = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--access-hook=\")) {\n-\t\t\taccess_hook = arg + 14;\n+\t\tif ((optarg = skip_prefix(arg, \"--access-hook=\")) != NULL) {\n+\t\t\taccess_hook = optarg;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--timeout=\")) {\n-\t\t\ttimeout = atoi(arg+10);\n+\t\tif ((optarg = skip_prefix(arg, \"--timeout=\")) != NULL) {\n+\t\t\ttimeout = atoi(optarg);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--init-timeout=\")) {\n-\t\t\tinit_timeout = atoi(arg+15);\n+\t\tif ((optarg = skip_prefix(arg, \"--init-timeout=\")) != NULL) {\n+\t\t\tinit_timeout = atoi(optarg);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--max-connections=\")) {\n-\t\t\tmax_connections = atoi(arg+18);\n+\t\tif ((optarg = skip_prefix(arg, \"--max-connections=\")) != NULL) {\n+\t\t\tmax_connections = atoi(optarg);\n \t\t\tif (max_connections < 0)\n \t\t\t\tmax_connections = 0;\t /* unlimited */\n \t\t\tcontinue;\n@@ -1221,16 +1222,16 @@ int main(int argc, char **argv)\n \t\t\tstrict_paths = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--base-path=\")) {\n-\t\t\tbase_path = arg+12;\n+\t\tif ((optarg = skip_prefix(arg, \"--base-path=\")) != NULL) {\n+\t\t\tbase_path = optarg;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--base-path-relaxed\")) {\n \t\t\tbase_path_relaxed = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--interpolated-path=\")) {\n-\t\t\tinterpolated_path = arg+20;\n+\t\tif ((optarg = skip_prefix(arg, \"--interpolated-path=\")) != NULL) {\n+\t\t\tinterpolated_path = optarg;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--reuseaddr\")) {\n@@ -1241,12 +1242,12 @@ int main(int argc, char **argv)\n \t\t\tuser_path = \"\";\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--user-path=\")) {\n-\t\t\tuser_path = arg + 12;\n+\t\tif ((optarg = skip_prefix(arg, \"--user-path=\")) != NULL) {\n+\t\t\tuser_path = optarg;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--pid-file=\")) {\n-\t\t\tpid_file = arg + 11;\n+\t\tif ((optarg = skip_prefix(arg, \"--pid-file=\")) != NULL) {\n+\t\t\tpid_file = optarg;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--detach\")) {\n@@ -1254,35 +1255,35 @@ int main(int argc, char **argv)\n \t\t\tlog_syslog = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--user=\")) {\n-\t\t\tuser_name = arg + 7;\n+\t\tif ((optarg = skip_prefix(arg, \"--user=\")) != NULL) {\n+\t\t\tuser_name = optarg;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--group=\")) {\n-\t\t\tgroup_name = arg + 8;\n+\t\tif ((optarg = skip_prefix(arg, \"--group=\")) != NULL) {\n+\t\t\tgroup_name = optarg;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--enable=\")) {\n-\t\t\tenable_service(arg + 9, 1);\n+\t\tif ((optarg = skip_prefix(arg, \"--enable=\")) != NULL) {\n+\t\t\tenable_service(optarg, 1);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--disable=\")) {\n-\t\t\tenable_service(arg + 10, 0);\n+\t\tif ((optarg = skip_prefix(arg, \"--disable=\")) != NULL) {\n+\t\t\tenable_service(optarg, 0);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--allow-override=\")) {\n-\t\t\tmake_service_overridable(arg + 17, 1);\n+\t\tif ((optarg = skip_prefix(arg, \"--allow-override=\")) != NULL) {\n+\t\t\tmake_service_overridable(optarg, 1);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--forbid-override=\")) {\n-\t\t\tmake_service_overridable(arg + 18, 0);\n+\t\tif ((optarg = skip_prefix(arg, \"--forbid-override=\")) != NULL) {\n+\t\t\tmake_service_overridable(optarg, 0);\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--informative-errors\")) {\n+\t\tif ((optarg = skip_prefix(arg, \"--informative-errors\")) != NULL) {\n \t\t\tinformative_errors = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--no-informative-errors\")) {\n+\t\tif ((optarg = skip_prefix(arg, \"--no-informative-errors\")) != NULL) {\n \t\t\tinformative_errors = 0;\n \t\t\tcontinue;\n \t\t}\ndiff --git a/diff.c b/diff.c\nindex b79432b..90a1929 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -2353,6 +2353,7 @@ static void builtin_diff(const char *name_a,\n \t\txdemitconf_t xecfg;\n \t\tstruct emit_callback ecbdata;\n \t\tconst struct userdiff_funcname *pe;\n+\t\tconst char *optarg;\n \n \t\tif (must_show_header) {\n \t\t\tfprintf(o->file, \"%s\", header.buf);\n@@ -2387,10 +2388,10 @@ static void builtin_diff(const char *name_a,\n \t\t\txdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);\n \t\tif (!diffopts)\n \t\t\t;\n-\t\telse if (starts_with(diffopts, \"--unified=\"))\n-\t\t\txecfg.ctxlen = strtoul(diffopts + 10, NULL, 10);\n-\t\telse if (starts_with(diffopts, \"-u\"))\n-\t\t\txecfg.ctxlen = strtoul(diffopts + 2, NULL, 10);\n+\t\telse if ((optarg = skip_prefix(diffopts, \"--unified=\")) != NULL)\n+\t\t\txecfg.ctxlen = strtoul(optarg, NULL, 10);\n+\t\telse if ((optarg = skip_prefix(diffopts, \"-u\")) != NULL)\n+\t\t\txecfg.ctxlen = strtoul(optarg, NULL, 10);\n \t\tif (o->word_diff)\n \t\t\tinit_diff_words_data(&ecbdata, o, one, two);\n \t\txdi_diff_outf(&mf1, &mf2, fn_out_consume, &ecbdata,\n@@ -3614,17 +3615,17 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->output_format |= DIFF_FORMAT_SHORTSTAT;\n \telse if (!strcmp(arg, \"-X\") || !strcmp(arg, \"--dirstat\"))\n \t\treturn parse_dirstat_opt(options, \"\");\n-\telse if (starts_with(arg, \"-X\"))\n-\t\treturn parse_dirstat_opt(options, arg + 2);\n-\telse if (starts_with(arg, \"--dirstat=\"))\n-\t\treturn parse_dirstat_opt(options, arg + 10);\n+\telse if ((optarg = skip_prefix(arg, \"-X\")) != NULL)\n+\t\treturn parse_dirstat_opt(options, optarg);\n+\telse if ((optarg = skip_prefix(arg, \"--dirstat=\")) != NULL)\n+\t\treturn parse_dirstat_opt(options, optarg);\n \telse if (!strcmp(arg, \"--cumulative\"))\n \t\treturn parse_dirstat_opt(options, \"cumulative\");\n \telse if (!strcmp(arg, \"--dirstat-by-file\"))\n \t\treturn parse_dirstat_opt(options, \"files\");\n-\telse if (starts_with(arg, \"--dirstat-by-file=\")) {\n+\telse if ((optarg = skip_prefix(arg, \"--dirstat-by-file=\")) != NULL) {\n \t\tparse_dirstat_opt(options, \"files\");\n-\t\treturn parse_dirstat_opt(options, arg + 18);\n+\t\treturn parse_dirstat_opt(options, optarg);\n \t}\n \telse if (!strcmp(arg, \"--check\"))\n \t\toptions->output_format |= DIFF_FORMAT_CHECKDIFF;\n@@ -3674,9 +3675,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\tDIFF_OPT_CLR(options, RENAME_EMPTY);\n \telse if (!strcmp(arg, \"--relative\"))\n \t\tDIFF_OPT_SET(options, RELATIVE_NAME);\n-\telse if (starts_with(arg, \"--relative=\")) {\n+\telse if ((optarg = skip_prefix(arg, \"--relative=\")) != NULL) {\n \t\tDIFF_OPT_SET(options, RELATIVE_NAME);\n-\t\toptions->prefix = arg + 11;\n+\t\toptions->prefix = optarg;\n \t}\n \n \t/* xdiff options */\n@@ -3727,8 +3728,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\tDIFF_OPT_CLR(options, FOLLOW_RENAMES);\n \telse if (!strcmp(arg, \"--color\"))\n \t\toptions->use_color = 1;\n-\telse if (starts_with(arg, \"--color=\")) {\n-\t\tint value = git_config_colorbool(NULL, arg+8);\n+\telse if ((optarg = skip_prefix(arg, \"--color=\")) != NULL) {\n+\t\tint value = git_config_colorbool(NULL, optarg);\n \t\tif (value < 0)\n \t\t\treturn error(\"option `color' expects \\\"always\\\", \\\"auto\\\", or \\\"never\\\"\");\n \t\toptions->use_color = value;\n@@ -3739,17 +3740,17 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\toptions->use_color = 1;\n \t\toptions->word_diff = DIFF_WORDS_COLOR;\n \t}\n-\telse if (starts_with(arg, \"--color-words=\")) {\n+\telse if ((optarg = skip_prefix(arg, \"--color-words=\")) != NULL) {\n \t\toptions->use_color = 1;\n \t\toptions->word_diff = DIFF_WORDS_COLOR;\n-\t\toptions->word_regex = arg + 14;\n+\t\toptions->word_regex = optarg;\n \t}\n \telse if (!strcmp(arg, \"--word-diff\")) {\n \t\tif (options->word_diff == DIFF_WORDS_NONE)\n \t\t\toptions->word_diff = DIFF_WORDS_PLAIN;\n \t}\n-\telse if (starts_with(arg, \"--word-diff=\")) {\n-\t\tconst char *type = arg + 12;\n+\telse if ((optarg = skip_prefix(arg, \"--word-diff=\")) != NULL) {\n+\t\tconst char *type = optarg;\n \t\tif (!strcmp(type, \"plain\"))\n \t\t\toptions->word_diff = DIFF_WORDS_PLAIN;\n \t\telse if (!strcmp(type, \"color\")) {\n@@ -3784,13 +3785,13 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \telse if (!strcmp(arg, \"--ignore-submodules\")) {\n \t\tDIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);\n \t\thandle_ignore_submodules_arg(options, \"all\");\n-\t} else if (starts_with(arg, \"--ignore-submodules=\")) {\n+\t} else if ((optarg = skip_prefix(arg, \"--ignore-submodules=\")) != NULL) {\n \t\tDIFF_OPT_SET(options, OVERRIDE_SUBMODULE_CONFIG);\n-\t\thandle_ignore_submodules_arg(options, arg + 20);\n+\t\thandle_ignore_submodules_arg(options, optarg);\n \t} else if (!strcmp(arg, \"--submodule\"))\n \t\tDIFF_OPT_SET(options, SUBMODULE_LOG);\n-\telse if (starts_with(arg, \"--submodule=\"))\n-\t\treturn parse_submodule_opt(options, arg + 12);\n+\telse if ((optarg = skip_prefix(arg, \"--submodule=\")) != NULL)\n+\t\treturn parse_submodule_opt(options, optarg);\n \n \t/* misc options */\n \telse if (!strcmp(arg, \"-z\"))\n@@ -3825,8 +3826,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t}\n \telse if (!strcmp(arg, \"--abbrev\"))\n \t\toptions->abbrev = DEFAULT_ABBREV;\n-\telse if (starts_with(arg, \"--abbrev=\")) {\n-\t\toptions->abbrev = strtoul(arg + 9, NULL, 10);\n+\telse if ((optarg = skip_prefix(arg, \"--abbrev=\")) != NULL) {\n+\t\toptions->abbrev = strtoul(optarg, NULL, 10);\n \t\tif (options->abbrev < MINIMUM_ABBREV)\n \t\t\toptions->abbrev = MINIMUM_ABBREV;\n \t\telse if (40 < options->abbrev)\ndiff --git a/git.c b/git.c\nindex 3799514..35fda7e 100644\n--- a/git.c\n+++ b/git.c\n@@ -40,6 +40,7 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \n \twhile (*argc > 0) {\n \t\tconst char *cmd = (*argv)[0];\n+\t\tconst char *optarg;\n \t\tif (cmd[0] != '-')\n \t\t\tbreak;\n \n@@ -92,8 +93,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \t\t\t\t*envchanged = 1;\n \t\t\t(*argv)++;\n \t\t\t(*argc)--;\n-\t\t} else if (starts_with(cmd, \"--git-dir=\")) {\n-\t\t\tsetenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);\n+\t\t} else if ((optarg = skip_prefix(cmd, \"--git-dir=\")) != NULL) {\n+\t\t\tsetenv(GIT_DIR_ENVIRONMENT, optarg, 1);\n \t\t\tif (envchanged)\n \t\t\t\t*envchanged = 1;\n \t\t} else if (!strcmp(cmd, \"--namespace\")) {\n@@ -106,8 +107,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \t\t\t\t*envchanged = 1;\n \t\t\t(*argv)++;\n \t\t\t(*argc)--;\n-\t\t} else if (starts_with(cmd, \"--namespace=\")) {\n-\t\t\tsetenv(GIT_NAMESPACE_ENVIRONMENT, cmd + 12, 1);\n+\t\t} else if ((optarg = skip_prefix(cmd, \"--namespace=\")) != NULL) {\n+\t\t\tsetenv(GIT_NAMESPACE_ENVIRONMENT, optarg, 1);\n \t\t\tif (envchanged)\n \t\t\t\t*envchanged = 1;\n \t\t} else if (!strcmp(cmd, \"--work-tree\")) {\n@@ -120,8 +121,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)\n \t\t\t\t*envchanged = 1;\n \t\t\t(*argv)++;\n \t\t\t(*argc)--;\n-\t\t} else if (starts_with(cmd, \"--work-tree=\")) {\n-\t\t\tsetenv(GIT_WORK_TREE_ENVIRONMENT, cmd + 12, 1);\n+\t\t} else if ((optarg = skip_prefix(cmd, \"--work-tree=\")) != NULL) {\n+\t\t\tsetenv(GIT_WORK_TREE_ENVIRONMENT, optarg, 1);\n \t\t\tif (envchanged)\n \t\t\t\t*envchanged = 1;\n \t\t} else if (!strcmp(cmd, \"--bare\")) {\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex a18bd15..ba7ecb6 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -2055,6 +2055,7 @@ void init_merge_options(struct merge_options *o)\n \n int parse_merge_opt(struct merge_options *o, const char *s)\n {\n+\tconst char *optarg;\n \tif (!s || !*s)\n \t\treturn -1;\n \tif (!strcmp(s, \"ours\"))\n@@ -2063,14 +2064,14 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n \t\to->recursive_variant = MERGE_RECURSIVE_THEIRS;\n \telse if (!strcmp(s, \"subtree\"))\n \t\to->subtree_shift = \"\";\n-\telse if (starts_with(s, \"subtree=\"))\n-\t\to->subtree_shift = s + strlen(\"subtree=\");\n+\telse if ((optarg = skip_prefix(s, \"subtree=\")) != NULL)\n+\t\to->subtree_shift = optarg;\n \telse if (!strcmp(s, \"patience\"))\n \t\to->xdl_opts = DIFF_WITH_ALG(o, PATIENCE_DIFF);\n \telse if (!strcmp(s, \"histogram\"))\n \t\to->xdl_opts = DIFF_WITH_ALG(o, HISTOGRAM_DIFF);\n-\telse if (starts_with(s, \"diff-algorithm=\")) {\n-\t\tlong value = parse_algorithm_value(s + strlen(\"diff-algorithm=\"));\n+\telse if ((optarg = skip_prefix(s, \"diff-algorithm=\")) != NULL) {\n+\t\tlong value = parse_algorithm_value(optarg);\n \t\tif (value < 0)\n \t\t\treturn -1;\n \t\t/* clear out previous settings */\n@@ -2088,8 +2089,8 @@ int parse_merge_opt(struct merge_options *o, const char *s)\n \t\to->renormalize = 1;\n \telse if (!strcmp(s, \"no-renormalize\"))\n \t\to->renormalize = 0;\n-\telse if (starts_with(s, \"rename-threshold=\")) {\n-\t\tconst char *score = s + strlen(\"rename-threshold=\");\n+\telse if ((optarg = skip_prefix(s, \"rename-threshold=\")) != NULL) {\n+\t\tconst char *score = optarg;\n \t\tif ((o->rename_score = parse_rename_score(&score)) == -1 || *score != 0)\n \t\t\treturn -1;\n \t}\ndiff --git a/revision.c b/revision.c\nindex a68fde6..94b66e8 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1652,8 +1652,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->max_count = atoi(argv[1]);\n \t\trevs->no_walk = 0;\n \t\treturn 2;\n-\t} else if (starts_with(arg, \"-n\")) {\n-\t\trevs->max_count = atoi(arg + 2);\n+\t} else if ((optarg = skip_prefix(arg, \"-n\")) != NULL) {\n+\t\trevs->max_count = atoi(optarg);\n \t\trevs->no_walk = 0;\n \t} else if ((argcount = parse_long_opt(\"max-age\", argv, &optarg))) {\n \t\trevs->max_age = atoi(optarg);\n@@ -1712,11 +1712,11 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t} else if (!strcmp(arg, \"--author-date-order\")) {\n \t\trevs->sort_order = REV_SORT_BY_AUTHOR_DATE;\n \t\trevs->topo_order = 1;\n-\t} else if (starts_with(arg, \"--early-output\")) {\n+\t} else if ((optarg = skip_prefix(arg, \"--early-output\")) != NULL) {\n \t\tint count = 100;\n-\t\tswitch (arg[14]) {\n+\t\tswitch (optarg[0]) {\n \t\tcase '=':\n-\t\t\tcount = atoi(arg+15);\n+\t\t\tcount = atoi(optarg + 1);\n \t\t\t/* Fallthrough */\n \t\tcase 0:\n \t\t\trevs->topo_order = 1;\n@@ -1737,12 +1737,12 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->min_parents = 2;\n \t} else if (!strcmp(arg, \"--no-merges\")) {\n \t\trevs->max_parents = 1;\n-\t} else if (starts_with(arg, \"--min-parents=\")) {\n-\t\trevs->min_parents = atoi(arg+14);\n+\t} else if ((optarg = skip_prefix(arg, \"--min-parents=\")) != NULL) {\n+\t\trevs->min_parents = atoi(optarg);\n \t} else if (starts_with(arg, \"--no-min-parents\")) {\n \t\trevs->min_parents = 0;\n-\t} else if (starts_with(arg, \"--max-parents=\")) {\n-\t\trevs->max_parents = atoi(arg+14);\n+\t} else if ((optarg = skip_prefix(arg, \"--max-parents=\")) != NULL) {\n+\t\trevs->max_parents = atoi(optarg);\n \t} else if (starts_with(arg, \"--no-max-parents\")) {\n \t\trevs->max_parents = -1;\n \t} else if (!strcmp(arg, \"--boundary\")) {\n@@ -1818,32 +1818,30 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->verbose_header = 1;\n \t\trevs->pretty_given = 1;\n \t\tget_commit_format(arg+8, revs);\n-\t} else if (starts_with(arg, \"--pretty=\") || starts_with(arg, \"--format=\")) {\n+\t} else if ((optarg = skip_prefix(arg, \"--pretty=\")) != NULL ||\n+\t\t (optarg = skip_prefix(arg, \"--format=\")) != NULL) {\n \t\t/*\n \t\t * Detached form (\"--pretty X\" as opposed to \"--pretty=X\")\n \t\t * not allowed, since the argument is optional.\n \t\t */\n \t\trevs->verbose_header = 1;\n \t\trevs->pretty_given = 1;\n-\t\tget_commit_format(arg+9, revs);\n+\t\tget_commit_format(optarg, revs);\n \t} else if (!strcmp(arg, \"--show-notes\") || !strcmp(arg, \"--notes\")) {\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n \t\trevs->notes_opt.use_default_notes = 1;\n \t} else if (!strcmp(arg, \"--show-signature\")) {\n \t\trevs->show_signature = 1;\n-\t} else if (starts_with(arg, \"--show-notes=\") ||\n-\t\t starts_with(arg, \"--notes=\")) {\n+\t} else if ((optarg = skip_prefix(arg, \"--show-notes=\")) != NULL ||\n+\t\t (optarg = skip_prefix(arg, \"--notes=\")) != NULL) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n \t\trevs->show_notes = 1;\n \t\trevs->show_notes_given = 1;\n-\t\tif (starts_with(arg, \"--show-notes\")) {\n-\t\t\tif (revs->notes_opt.use_default_notes < 0)\n-\t\t\t\trevs->notes_opt.use_default_notes = 1;\n-\t\t\tstrbuf_addstr(&buf, arg+13);\n-\t\t}\n-\t\telse\n-\t\t\tstrbuf_addstr(&buf, arg+8);\n+\t\tif (starts_with(arg, \"--show-notes\") &&\n+\t\t revs->notes_opt.use_default_notes < 0)\n+\t\t\trevs->notes_opt.use_default_notes = 1;\n+\t\tstrbuf_addstr(&buf, optarg);\n \t\texpand_notes_ref(&buf);\n \t\tstring_list_append(&revs->notes_opt.extra_notes_refs,\n \t\t\t\t strbuf_detach(&buf, NULL));\n@@ -1880,8 +1878,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->abbrev = 0;\n \t} else if (!strcmp(arg, \"--abbrev\")) {\n \t\trevs->abbrev = DEFAULT_ABBREV;\n-\t} else if (starts_with(arg, \"--abbrev=\")) {\n-\t\trevs->abbrev = strtoul(arg + 9, NULL, 10);\n+\t} else if ((optarg = skip_prefix(arg, \"--abbrev=\")) != NULL) {\n+\t\trevs->abbrev = strtoul(optarg, NULL, 10);\n \t\tif (revs->abbrev < MINIMUM_ABBREV)\n \t\t\trevs->abbrev = MINIMUM_ABBREV;\n \t\telse if (revs->abbrev > 40)\n@@ -2027,20 +2025,20 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \t} else if ((argcount = parse_long_opt(\"exclude\", argv, &optarg))) {\n \t\tadd_ref_exclusion(&revs->ref_excludes, optarg);\n \t\treturn argcount;\n-\t} else if (starts_with(arg, \"--branches=\")) {\n+\t} else if ((optarg = skip_prefix(arg, \"--branches=\")) != NULL) {\n \t\tstruct all_refs_cb cb;\n \t\tinit_all_refs_cb(&cb, revs, *flags);\n-\t\tfor_each_glob_ref_in(handle_one_ref, arg + 11, \"refs/heads/\", &cb);\n+\t\tfor_each_glob_ref_in(handle_one_ref, optarg, \"refs/heads/\", &cb);\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n-\t} else if (starts_with(arg, \"--tags=\")) {\n+\t} else if ((optarg = skip_prefix(arg, \"--tags=\")) != NULL) {\n \t\tstruct all_refs_cb cb;\n \t\tinit_all_refs_cb(&cb, revs, *flags);\n-\t\tfor_each_glob_ref_in(handle_one_ref, arg + 7, \"refs/tags/\", &cb);\n+\t\tfor_each_glob_ref_in(handle_one_ref, optarg, \"refs/tags/\", &cb);\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n-\t} else if (starts_with(arg, \"--remotes=\")) {\n+\t} else if ((optarg = skip_prefix(arg, \"--remotes=\")) != NULL) {\n \t\tstruct all_refs_cb cb;\n \t\tinit_all_refs_cb(&cb, revs, *flags);\n-\t\tfor_each_glob_ref_in(handle_one_ref, arg + 10, \"refs/remotes/\", &cb);\n+\t\tfor_each_glob_ref_in(handle_one_ref, optarg, \"refs/remotes/\", &cb);\n \t\tclear_ref_exclusion(&revs->ref_excludes);\n \t} else if (!strcmp(arg, \"--reflog\")) {\n \t\thandle_reflog(revs, *flags);\n@@ -2048,14 +2046,14 @@ static int handle_revision_pseudo_opt(const char *submodule,\n \t\t*flags ^= UNINTERESTING | BOTTOM;\n \t} else if (!strcmp(arg, \"--no-walk\")) {\n \t\trevs->no_walk = REVISION_WALK_NO_WALK_SORTED;\n-\t} else if (starts_with(arg, \"--no-walk=\")) {\n+\t} else if ((optarg = skip_prefix(arg, \"--no-walk=\")) != NULL) {\n \t\t/*\n \t\t * Detached form (\"--no-walk X\" as opposed to \"--no-walk=X\")\n \t\t * not allowed, since the argument is optional.\n \t\t */\n-\t\tif (!strcmp(arg + 10, \"sorted\"))\n+\t\tif (!strcmp(optarg, \"sorted\"))\n \t\t\trevs->no_walk = REVISION_WALK_NO_WALK_SORTED;\n-\t\telse if (!strcmp(arg + 10, \"unsorted\"))\n+\t\telse if (!strcmp(optarg, \"unsorted\"))\n \t\t\trevs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n \t\telse\n \t\t\treturn error(\"invalid argument to --no-walk\");\ndiff --git a/upload-pack.c b/upload-pack.c\nindex ec56cdb..8523b42 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -799,6 +799,7 @@ int main(int argc, char **argv)\n \n \tfor (i = 1; i < argc; i++) {\n \t\tchar *arg = argv[i];\n+\t\tconst char *optarg;\n \n \t\tif (arg[0] != '-')\n \t\t\tbreak;\n@@ -814,8 +815,8 @@ int main(int argc, char **argv)\n \t\t\tstrict = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--timeout=\")) {\n-\t\t\ttimeout = atoi(arg+10);\n+\t\tif ((optarg = skip_prefix(arg, \"--timeout=\")) != NULL) {\n+\t\t\ttimeout = atoi(optarg);\n \t\t\tdaemon_mode = 1;\n \t\t\tcontinue;\n \t\t}\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232156","messageId":"1387378437-20646-4-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 03/12] Add and use skip_prefix_defval()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:48Z","receivedAt":"2013-12-18T14:53:48Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This is a variant of skip_prefix() that returns a specied pointer\ninstead of NULL if no prefix is found. It's helpful to simplify\n\n if (starts_with(foo, \"bar\"))\n foo += 3;\n\ninto\n\n foo = skip_prefix_gently(foo, \"bar\", foo);\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/checkout.c | 6 ++----\n builtin/fast-export.c | 3 +--\n builtin/merge.c | 4 ++--\n builtin/show-branch.c | 14 +++++---------\n git-compat-util.h | 9 +++++++--\n git.c | 3 +--\n notes.c | 6 ++----\n wt-status.c | 12 ++++--------\n 8 files changed, 24 insertions(+), 33 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 5df3837..6531ed4 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1151,10 +1151,8 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\tconst char *argv0 = argv[0];\n \t\tif (!argc || !strcmp(argv0, \"--\"))\n \t\t\tdie (_(\"--track needs a branch name\"));\n-\t\tif (starts_with(argv0, \"refs/\"))\n-\t\t\targv0 += 5;\n-\t\tif (starts_with(argv0, \"remotes/\"))\n-\t\t\targv0 += 8;\n+\t\targv0 = skip_prefix_defval(argv0, \"refs/\", argv0);\n+\t\targv0 = skip_prefix_defval(argv0, \"remotes/\", argv0);\n \t\targv0 = strchr(argv0, '/');\n \t\tif (!argv0 || !argv0[1])\n \t\t\tdie (_(\"Missing branch name; try -b\"));\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex b8d8a3a..cd0a302 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -476,8 +476,7 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\t}\n \t}\n \n-\tif (starts_with(name, \"refs/tags/\"))\n-\t\tname += 10;\n+\tname = skip_prefix_defval(name, \"refs/tags/\", name);\n \tprintf(\"tag %s\\nfrom :%d\\n%.*s%sdata %d\\n%.*s\\n\",\n \t name, tagged_mark,\n \t (int)(tagger_end - tagger), tagger,\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 4941a6c..590d907 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1106,8 +1106,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t * current branch.\n \t */\n \tbranch = branch_to_free = resolve_refdup(\"HEAD\", head_sha1, 0, &flag);\n-\tif (branch && starts_with(branch, \"refs/heads/\"))\n-\t\tbranch += 11;\n+\tif (branch)\n+\t\tbranch = skip_prefix_defval(branch, \"refs/heads/\", branch);\n \tif (!branch || is_null_sha1(head_sha1))\n \t\thead_commit = NULL;\n \telse\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex d9217ce..6078132 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -284,8 +284,7 @@ static void show_one_commit(struct commit *commit, int no_name)\n \t\tpp_commit_easy(CMIT_FMT_ONELINE, commit, &pretty);\n \t\tpretty_str = pretty.buf;\n \t}\n-\tif (starts_with(pretty_str, \"[PATCH] \"))\n-\t\tpretty_str += 8;\n+\tpretty_str = skip_prefix_defval(pretty_str, \"[PATCH] \", pretty_str);\n \n \tif (!no_name) {\n \t\tif (name && name->head_name) {\n@@ -473,14 +472,13 @@ static void snarf_refs(int head, int remotes)\n \t}\n }\n \n-static int rev_is_head(char *head, int headlen, char *name,\n+static int rev_is_head(const char *head, int headlen, const char *name,\n \t\t unsigned char *head_sha1, unsigned char *sha1)\n {\n \tif ((!head[0]) ||\n \t (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))\n \t\treturn 0;\n-\tif (starts_with(head, \"refs/heads/\"))\n-\t\thead += 11;\n+\thead = skip_prefix_defval(head, \"refs/heads/\", head);\n \tif (starts_with(name, \"refs/heads/\"))\n \t\tname += 11;\n \telse if (starts_with(name, \"heads/\"))\n@@ -811,10 +809,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\t\thead_sha1, NULL))\n \t\t\t\thas_head++;\n \t\t}\n-\t\tif (!has_head) {\n-\t\t\tint offset = starts_with(head, \"refs/heads/\") ? 11 : 0;\n-\t\t\tappend_one_rev(head + offset);\n-\t\t}\n+\t\tif (!has_head)\n+\t\t\tappend_one_rev(skip_prefix_defval(head, \"refs/heads/\", head));\n \t}\n \n \tif (!ref_name_cnt) {\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 84f1078..b72a80d 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -354,10 +354,15 @@ extern int prefixcmp(const char *str, const char *prefix);\n extern int ends_with(const char *str, const char *suffix);\n extern int suffixcmp(const char *str, const char *suffix);\n \n-static inline const char *skip_prefix(const char *str, const char *prefix)\n+static inline const char *skip_prefix_defval(const char *str, const char *prefix, const char *defval)\n {\n \tsize_t len = strlen(prefix);\n-\treturn strncmp(str, prefix, len) ? NULL : str + len;\n+\treturn strncmp(str, prefix, len) ? defval : str + len;\n+}\n+\n+static inline const char *skip_prefix(const char *str, const char *prefix)\n+{\n+\treturn skip_prefix_defval(str, prefix, NULL);\n }\n \n static inline int starts_with(const char *str, const char *prefix)\ndiff --git a/git.c b/git.c\nindex 35fda7e..321ae81 100644\n--- a/git.c\n+++ b/git.c\n@@ -579,8 +579,7 @@ int main(int argc, char **av)\n \targc--;\n \thandle_options(&argv, &argc, NULL);\n \tif (argc > 0) {\n-\t\tif (starts_with(argv[0], \"--\"))\n-\t\t\targv[0] += 2;\n+\t\targv[0] = skip_prefix_defval(argv[0], \"--\", argv[0]);\n \t} else {\n \t\t/* The user didn't specify a command; give them help */\n \t\tcommit_pager_choice();\ndiff --git a/notes.c b/notes.c\nindex 5f07c0b..31f513b 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -1243,10 +1243,8 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,\n \t\tif (!ref || !strcmp(ref, GIT_NOTES_DEFAULT_REF)) {\n \t\t\tstrbuf_addstr(sb, \"\\nNotes:\\n\");\n \t\t} else {\n-\t\t\tif (starts_with(ref, \"refs/\"))\n-\t\t\t\tref += 5;\n-\t\t\tif (starts_with(ref, \"notes/\"))\n-\t\t\t\tref += 6;\n+\t\t\tref = skip_prefix_defval(ref, \"refs/\", ref);\n+\t\t\tref = skip_prefix_defval(ref, \"notes/\", ref);\n \t\t\tstrbuf_addf(sb, \"\\nNotes (%s):\\n\", ref);\n \t\t}\n \t}\ndiff --git a/wt-status.c b/wt-status.c\nindex 4e55810..1f65039 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1179,14 +1179,10 @@ static void wt_status_get_detached_from(struct wt_status_state *state)\n \t /* perhaps sha1 is a tag, try to dereference to a commit */\n \t ((commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&\n \t !hashcmp(cb.nsha1, commit->object.sha1)))) {\n-\t\tint ofs;\n-\t\tif (starts_with(ref, \"refs/tags/\"))\n-\t\t\tofs = strlen(\"refs/tags/\");\n-\t\telse if (starts_with(ref, \"refs/remotes/\"))\n-\t\t\tofs = strlen(\"refs/remotes/\");\n-\t\telse\n-\t\t\tofs = 0;\n-\t\tstate->detached_from = xstrdup(ref + ofs);\n+\t\tconst char *p;\n+\t\tif ((p = skip_prefix_defval(ref, \"refs/tags/\", ref)) == ref)\n+\t\t\tp = skip_prefix_defval(ref, \"refs/remotes/\", ref);\n+\t\tstate->detached_from = xstrdup(p);\n \t} else\n \t\tstate->detached_from =\n \t\t\txstrdup(find_unique_abbrev(cb.nsha1, DEFAULT_ABBREV));\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232155","messageId":"1387378437-20646-5-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 04/12] Replace some use of starts_with() with skip_prefix()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:49Z","receivedAt":"2013-12-18T14:53:49Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"All the changes follow the pattern\n\n if (!starts_with(foo, \"bar\"))\n return;\n foo += 3;\n\nwhich is turned into\n\n if ((foo = skip_prefix(foo, \"bar\")) == NULL)\n return;\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/branch.c | 3 +--\n pretty.c | 3 +--\n setup.c | 3 +--\n tag.c | 7 +++----\n wt-status.c | 3 +--\n 5 files changed, 7 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex b4d7716..d063de2 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -868,9 +868,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)\n \tif (!strcmp(head, \"HEAD\")) {\n \t\tdetached = 1;\n \t} else {\n-\t\tif (!starts_with(head, \"refs/heads/\"))\n+\t\tif ((head = skip_prefix(head, \"refs/heads/\")) == NULL)\n \t\t\tdie(_(\"HEAD not found below refs/heads!\"));\n-\t\thead += 11;\n \t}\n \thashcpy(merge_filter_ref, head_sha1);\n \ndiff --git a/pretty.c b/pretty.c\nindex 87db08b..08e30ec 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -40,10 +40,9 @@ static int git_pretty_formats_config(const char *var, const char *value, void *c\n \tconst char *fmt;\n \tint i;\n \n-\tif (!starts_with(var, \"pretty.\"))\n+\tif ((name = skip_prefix(var, \"pretty.\")) == NULL)\n \t\treturn 0;\n \n-\tname = var + strlen(\"pretty.\");\n \tfor (i = 0; i < builtin_formats_len; i++) {\n \t\tif (!strcmp(commit_formats[i].name, name))\n \t\t\treturn 0;\ndiff --git a/setup.c b/setup.c\nindex 6c3f85f..debfaab 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -304,14 +304,13 @@ const char *read_gitfile(const char *path)\n \tif (len != st.st_size)\n \t\tdie(\"Error reading %s\", path);\n \tbuf[len] = '\\0';\n-\tif (!starts_with(buf, \"gitdir: \"))\n+\tif ((dir = (char *)skip_prefix(buf, \"gitdir: \")) == NULL)\n \t\tdie(\"Invalid gitfile format: %s\", path);\n \twhile (buf[len - 1] == '\\n' || buf[len - 1] == '\\r')\n \t\tlen--;\n \tif (len < 9)\n \t\tdie(\"No path in gitfile: %s\", path);\n \tbuf[len] = '\\0';\n-\tdir = buf + 8;\n \n \tif (!is_absolute_path(dir) && (slash = strrchr(path, '/'))) {\n \t\tsize_t pathlen = slash+1 - path;\ndiff --git a/tag.c b/tag.c\nindex 7b07921..9b63d1b 100644\n--- a/tag.c\n+++ b/tag.c\n@@ -86,9 +86,8 @@ int parse_tag_buffer(struct tag *item, const void *data, unsigned long size)\n \t\treturn -1;\n \tbufptr += 48; /* \"object \" + sha1 + \"\\n\" */\n \n-\tif (!starts_with(bufptr, \"type \"))\n+\tif ((bufptr = skip_prefix(bufptr, \"type \")) == NULL)\n \t\treturn -1;\n-\tbufptr += 5;\n \tnl = memchr(bufptr, '\\n', tail - bufptr);\n \tif (!nl || sizeof(type) <= (nl - bufptr))\n \t\treturn -1;\n@@ -109,11 +108,11 @@ int parse_tag_buffer(struct tag *item, const void *data, unsigned long size)\n \t\titem->tagged = NULL;\n \t}\n \n-\tif (bufptr + 4 < tail && starts_with(bufptr, \"tag \"))\n+\tif (bufptr + 4 < tail &&\n+\t (bufptr = skip_prefix(bufptr, \"tag \")) != NULL)\n \t\t; \t\t/* good */\n \telse\n \t\treturn -1;\n-\tbufptr += 4;\n \tnl = memchr(bufptr, '\\n', tail - bufptr);\n \tif (!nl)\n \t\treturn -1;\ndiff --git a/wt-status.c b/wt-status.c\nindex 1f65039..185fa81 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1145,9 +1145,8 @@ static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,\n \tstruct grab_1st_switch_cbdata *cb = cb_data;\n \tconst char *target = NULL, *end;\n \n-\tif (!starts_with(message, \"checkout: moving from \"))\n+\tif ((message = skip_prefix(message, \"checkout: moving from \")) == NULL)\n \t\treturn 0;\n-\tmessage += strlen(\"checkout: moving from \");\n \ttarget = strstr(message, \" to \");\n \tif (!target)\n \t\treturn 0;\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232158","messageId":"1387378437-20646-6-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 05/12] Convert a lot of starts_with() to skip_prefix()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:50Z","receivedAt":"2013-12-18T14:53:50Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The purpose is remove hard coded string length. Some could be a few\nlines away from the string comparison and easy to be missed when the\nstring is changed.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/for-each-ref.c | 9 +++++----\n builtin/mailinfo.c | 6 +++---\n builtin/merge.c | 8 +++++---\n builtin/remote.c | 3 +--\n commit.c | 5 +----\n diff.c | 9 +++------\n fetch-pack.c | 9 +++++----\n http-backend.c | 5 +++--\n http-push.c | 6 +++---\n http.c | 5 +++--\n log-tree.c | 5 +++--\n pager.c | 2 +-\n pathspec.c | 5 +++--\n refs.c | 12 +++++++-----\n sha1_name.c | 12 +++---------\n transport-helper.c | 15 +++++++--------\n transport.c | 14 ++++++++------\n 17 files changed, 64 insertions(+), 66 deletions(-)\n\ndiff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c\nindex 6551e7b..25c1388 100644\n--- a/builtin/for-each-ref.c\n+++ b/builtin/for-each-ref.c\n@@ -662,6 +662,7 @@ static void populate_value(struct refinfo *ref)\n \t\tconst char *refname;\n \t\tconst char *formatp;\n \t\tstruct branch *branch = NULL;\n+\t\tconst char *next;\n \n \t\tif (*name == '*') {\n \t\t\tderef = 1;\n@@ -674,18 +675,18 @@ static void populate_value(struct refinfo *ref)\n \t\t\trefname = ref->symref ? ref->symref : \"\";\n \t\telse if (starts_with(name, \"upstream\")) {\n \t\t\t/* only local branches may have an upstream */\n-\t\t\tif (!starts_with(ref->refname, \"refs/heads/\"))\n+\t\t\tif ((next = skip_prefix(ref->refname, \"refs/heads/\")) == NULL)\n \t\t\t\tcontinue;\n-\t\t\tbranch = branch_get(ref->refname + 11);\n+\t\t\tbranch = branch_get(next);\n \n \t\t\tif (!branch || !branch->merge || !branch->merge[0] ||\n \t\t\t !branch->merge[0]->dst)\n \t\t\t\tcontinue;\n \t\t\trefname = branch->merge[0]->dst;\n-\t\t} else if (starts_with(name, \"color:\")) {\n+\t\t} else if ((next = skip_prefix(name, \"color:\")) != NULL) {\n \t\t\tchar color[COLOR_MAXLEN] = \"\";\n \n-\t\t\tcolor_parse(name + 6, \"--format\", color);\n+\t\t\tcolor_parse(next, \"--format\", color);\n \t\t\tv->s = xstrdup(color);\n \t\t\tcontinue;\n \t\t} else if (!strcmp(name, \"flag\")) {\ndiff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\nindex 2100e23..daaafbd 100644\n--- a/builtin/mailinfo.c\n+++ b/builtin/mailinfo.c\n@@ -328,13 +328,13 @@ static int check_header(const struct strbuf *line,\n \t}\n \n \t/* for inbody stuff */\n-\tif (starts_with(line->buf, \">From\") && isspace(line->buf[5])) {\n+\tif (isspace(*skip_prefix_defval(line->buf, \">From\", \"NOSPACE\"))) {\n \t\tret = 1; /* Should this return 0? */\n \t\tgoto check_header_out;\n \t}\n-\tif (starts_with(line->buf, \"[PATCH]\") && isspace(line->buf[7])) {\n+\tif (isspace(*skip_prefix_defval(line->buf, \"[PATCH]\", \"NOSPACE\"))) {\n \t\tfor (i = 0; header[i]; i++) {\n-\t\t\tif (!memcmp(\"Subject\", header[i], 7)) {\n+\t\t\tif (starts_with(header[i], \"Subject\")) {\n \t\t\t\thandle_header(&hdr_data[i], line);\n \t\t\t\tret = 1;\n \t\t\t\tgoto check_header_out;\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 590d907..603f80a 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -569,10 +569,12 @@ static void parse_branch_merge_options(char *bmo)\n static int git_merge_config(const char *k, const char *v, void *cb)\n {\n \tint status;\n+\tconst char *kk, *kkk;\n \n-\tif (branch && starts_with(k, \"branch.\") &&\n-\t\tstarts_with(k + 7, branch) &&\n-\t\t!strcmp(k + 7 + strlen(branch), \".mergeoptions\")) {\n+\tif (branch &&\n+\t (kk = skip_prefix(k, \"branch.\")) != NULL &&\n+\t (kkk = skip_prefix(kk, branch)) != NULL &&\n+\t !strcmp(kkk, \".mergeoptions\")) {\n \t\tfree(branch_mergeoptions);\n \t\tbranch_mergeoptions = xstrdup(v);\n \t\treturn 0;\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex b3ab4cf..218c8c8 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -259,14 +259,13 @@ static const char *abbrev_ref(const char *name, const char *prefix)\n \n static int config_read_branches(const char *key, const char *value, void *cb)\n {\n-\tif (starts_with(key, \"branch.\")) {\n+\tif ((key = skip_prefix(key, \"branch.\")) != NULL) {\n \t\tconst char *orig_key = key;\n \t\tchar *name;\n \t\tstruct string_list_item *item;\n \t\tstruct branch_info *info;\n \t\tenum { REMOTE, MERGE, REBASE } type;\n \n-\t\tkey += 7;\n \t\tif (ends_with(key, \".remote\")) {\n \t\t\tname = xstrndup(key, strlen(key) - 7);\n \t\t\ttype = REMOTE;\ndiff --git a/commit.c b/commit.c\nindex 5df1df7..eed2ff9 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1193,10 +1193,7 @@ static void parse_gpg_output(struct signature_check *sigc)\n \tfor (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {\n \t\tconst char *found, *next;\n \n-\t\tif (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {\n-\t\t\t/* At the very beginning of the buffer */\n-\t\t\tfound = buf + strlen(sigcheck_gpg_status[i].check + 1);\n-\t\t} else {\n+\t\tif ((found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1)) == NULL) {\n \t\t\tfound = strstr(buf, sigcheck_gpg_status[i].check);\n \t\t\tif (!found)\n \t\t\t\tcontinue;\ndiff --git a/diff.c b/diff.c\nindex 90a1929..d754e2f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3388,13 +3388,10 @@ static inline int short_opt(char opt, const char **argv,\n int parse_long_opt(const char *opt, const char **argv,\n \t\t const char **optarg)\n {\n-\tconst char *arg = argv[0];\n-\tif (arg[0] != '-' || arg[1] != '-')\n-\t\treturn 0;\n-\targ += strlen(\"--\");\n-\tif (!starts_with(arg, opt))\n+\tconst char *arg;\n+\tif ((arg = skip_prefix(argv[0], \"--\")) == NULL ||\n+\t (arg = skip_prefix(arg, opt)) == NULL)\n \t\treturn 0;\n-\targ += strlen(opt);\n \tif (*arg == '=') { /* stuck form: --option=value */\n \t\t*optarg = arg + 1;\n \t\treturn 1;\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 760ed16..723ff06 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -317,18 +317,19 @@ static int find_common(struct fetch_pack_args *args,\n \n \tif (args->depth > 0) {\n \t\tchar *line;\n+\t\tconst char *sha1_str;\n \t\tunsigned char sha1[20];\n \n \t\tsend_request(args, fd[1], &req_buf);\n \t\twhile ((line = packet_read_line(fd[0], NULL))) {\n-\t\t\tif (starts_with(line, \"shallow \")) {\n-\t\t\t\tif (get_sha1_hex(line + 8, sha1))\n+\t\t\tif ((sha1_str = skip_prefix(line, \"shallow \")) != NULL) {\n+\t\t\t\tif (get_sha1_hex(sha1_str, sha1))\n \t\t\t\t\tdie(\"invalid shallow line: %s\", line);\n \t\t\t\tregister_shallow(sha1);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (starts_with(line, \"unshallow \")) {\n-\t\t\t\tif (get_sha1_hex(line + 10, sha1))\n+\t\t\tif ((sha1_str = skip_prefix(line, \"unshallow \")) != NULL) {\n+\t\t\t\tif (get_sha1_hex(sha1_str, sha1))\n \t\t\t\t\tdie(\"invalid unshallow line: %s\", line);\n \t\t\t\tif (!lookup_object(sha1))\n \t\t\t\t\tdie(\"object not found: %s\", line);\ndiff --git a/http-backend.c b/http-backend.c\nindex d2c0a62..e780c55 100644\n--- a/http-backend.c\n+++ b/http-backend.c\n@@ -221,17 +221,18 @@ static void get_idx_file(char *name)\n \n static int http_config(const char *var, const char *value, void *cb)\n {\n+\tconst char *p;\n \tif (!strcmp(var, \"http.getanyfile\")) {\n \t\tgetanyfile = git_config_bool(var, value);\n \t\treturn 0;\n \t}\n \n-\tif (starts_with(var, \"http.\")) {\n+\tif ((p = skip_prefix(var, \"http.\")) != NULL) {\n \t\tint i;\n \n \t\tfor (i = 0; i < ARRAY_SIZE(rpc_service); i++) {\n \t\t\tstruct rpc_service *svc = &rpc_service[i];\n-\t\t\tif (!strcmp(var + 5, svc->config_name)) {\n+\t\t\tif (!strcmp(p, svc->config_name)) {\n \t\t\t\tsvc->enabled = git_config_bool(var, value);\n \t\t\t\treturn 0;\n \t\t\t}\ndiff --git a/http-push.c b/http-push.c\nindex d4b40c9..5db6f28 100644\n--- a/http-push.c\n+++ b/http-push.c\n@@ -771,9 +771,9 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed)\n \t\t\tlock->owner = xmalloc(strlen(ctx->cdata) + 1);\n \t\t\tstrcpy(lock->owner, ctx->cdata);\n \t\t} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TIMEOUT)) {\n-\t\t\tif (starts_with(ctx->cdata, \"Second-\"))\n-\t\t\t\tlock->timeout =\n-\t\t\t\t\tstrtol(ctx->cdata + 7, NULL, 10);\n+\t\t\tconst char *p;\n+\t\t\tif ((p = skip_prefix(ctx->cdata, \"Second-\")) != NULL)\n+\t\t\t\tlock->timeout = strtol(p, NULL, 10);\n \t\t} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {\n \t\t\tlock->token = xmalloc(strlen(ctx->cdata) + 1);\n \t\t\tstrcpy(lock->token, ctx->cdata);\ndiff --git a/http.c b/http.c\nindex 70eaa26..1120ed2 100644\n--- a/http.c\n+++ b/http.c\n@@ -1098,6 +1098,7 @@ int http_fetch_ref(const char *base, struct ref *ref)\n \tchar *url;\n \tstruct strbuf buffer = STRBUF_INIT;\n \tint ret = -1;\n+\tconst char *p;\n \n \toptions.no_cache = 1;\n \n@@ -1106,8 +1107,8 @@ int http_fetch_ref(const char *base, struct ref *ref)\n \t\tstrbuf_rtrim(&buffer);\n \t\tif (buffer.len == 40)\n \t\t\tret = get_sha1_hex(buffer.buf, ref->old_sha1);\n-\t\telse if (starts_with(buffer.buf, \"ref: \")) {\n-\t\t\tref->symref = xstrdup(buffer.buf + 5);\n+\t\telse if ((p = skip_prefix(buffer.buf, \"ref: \")) != NULL) {\n+\t\t\tref->symref = xstrdup(p);\n \t\t\tret = 0;\n \t\t}\n \t}\ndiff --git a/log-tree.c b/log-tree.c\nindex 642faff..cef7c8d 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -96,13 +96,14 @@ static void add_name_decoration(enum decoration_type type, const char *name, str\n static int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)\n {\n \tstruct object *obj;\n+\tconst char *name;\n \tenum decoration_type type = DECORATION_NONE;\n \n-\tif (starts_with(refname, \"refs/replace/\")) {\n+\tif ((name = skip_prefix(refname, \"refs/replace/\")) != NULL) {\n \t\tunsigned char original_sha1[20];\n \t\tif (!read_replace_refs)\n \t\t\treturn 0;\n-\t\tif (get_sha1_hex(refname + 13, original_sha1)) {\n+\t\tif (get_sha1_hex(name, original_sha1)) {\n \t\t\twarning(\"invalid replace ref %s\", refname);\n \t\t\treturn 0;\n \t\t}\ndiff --git a/pager.c b/pager.c\nindex 345b0bc..175cd9f 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -151,7 +151,7 @@ int decimal_width(int number)\n static int pager_command_config(const char *var, const char *value, void *data)\n {\n \tstruct pager_config *c = data;\n-\tif (starts_with(var, \"pager.\") && !strcmp(var + 6, c->cmd)) {\n+\tif (!strcmp(skip_prefix_defval(var, \"pager.\", \"\"), c->cmd)) {\n \t\tint b = git_config_maybe_bool(var, value);\n \t\tif (b >= 0)\n \t\t\tc->want = b;\ndiff --git a/pathspec.c b/pathspec.c\nindex 52d38a4..e15f215 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -149,14 +149,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item,\n \t\t\tif (!len)\n \t\t\t\tcontinue;\n \t\t\tfor (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {\n+\t\t\t\tconst char *prefix_str;\n \t\t\t\tif (strlen(pathspec_magic[i].name) == len &&\n \t\t\t\t !strncmp(pathspec_magic[i].name, copyfrom, len)) {\n \t\t\t\t\tmagic |= pathspec_magic[i].bit;\n \t\t\t\t\tbreak;\n \t\t\t\t}\n-\t\t\t\tif (starts_with(copyfrom, \"prefix:\")) {\n+\t\t\t\tif ((prefix_str = skip_prefix(copyfrom, \"prefix:\")) != NULL) {\n \t\t\t\t\tchar *endptr;\n-\t\t\t\t\tpathspec_prefix = strtol(copyfrom + 7,\n+\t\t\t\t\tpathspec_prefix = strtol(prefix_str,\n \t\t\t\t\t\t\t\t &endptr, 10);\n \t\t\t\t\tif (endptr - copyfrom != len)\n \t\t\t\t\t\tdie(_(\"invalid parameter for pathspec magic 'prefix'\"));\ndiff --git a/refs.c b/refs.c\nindex 3926136..5e378bc 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1873,11 +1873,13 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)\n \n const char *prettify_refname(const char *name)\n {\n-\treturn name + (\n-\t\tstarts_with(name, \"refs/heads/\") ? 11 :\n-\t\tstarts_with(name, \"refs/tags/\") ? 10 :\n-\t\tstarts_with(name, \"refs/remotes/\") ? 13 :\n-\t\t0);\n+\tconst char *p;\n+\tif ((p = skip_prefix(name, \"refs/heads/\")) != NULL ||\n+\t (p = skip_prefix(name, \"refs/tags/\")) != NULL ||\n+\t (p = skip_prefix(name, \"refs/remotes/\")) != NULL)\n+\t\treturn p;\n+\telse\n+\t\treturn name;\n }\n \n const char *ref_rev_parse_rules[] = {\ndiff --git a/sha1_name.c b/sha1_name.c\nindex b1873d8..3fc4ede 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -546,14 +546,10 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n \t\tif (read_ref_at(real_ref, at_time, nth, sha1, NULL,\n \t\t\t\t&co_time, &co_tz, &co_cnt)) {\n \t\t\tif (!len) {\n-\t\t\t\tif (starts_with(real_ref, \"refs/heads/\")) {\n-\t\t\t\t\tstr = real_ref + 11;\n-\t\t\t\t\tlen = strlen(real_ref + 11);\n-\t\t\t\t} else {\n+\t\t\t\tif ((str = skip_prefix(real_ref, \"refs/heads/\")) == NULL)\n \t\t\t\t\t/* detached HEAD */\n \t\t\t\t\tstr = \"HEAD\";\n-\t\t\t\t\tlen = 4;\n-\t\t\t\t}\n+\t\t\t\tlen = strlen(str);\n \t\t\t}\n \t\t\tif (at_time)\n \t\t\t\twarning(\"Log for '%.*s' only goes \"\n@@ -909,10 +905,8 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,\n \tconst char *match = NULL, *target = NULL;\n \tsize_t len;\n \n-\tif (starts_with(message, \"checkout: moving from \")) {\n-\t\tmatch = message + strlen(\"checkout: moving from \");\n+\tif ((match = skip_prefix(message, \"checkout: moving from \")) != NULL)\n \t\ttarget = strstr(match, \" to \");\n-\t}\n \n \tif (!match || !target)\n \t\treturn 0;\ndiff --git a/transport-helper.c b/transport-helper.c\nindex 2010674..601aba8 100644\n--- a/transport-helper.c\n+++ b/transport-helper.c\n@@ -373,10 +373,10 @@ static int fetch_with_fetch(struct transport *transport,\n \tsendline(data, &buf);\n \n \twhile (1) {\n+\t\tconst char *name;\n \t\trecvline(data, &buf);\n \n-\t\tif (starts_with(buf.buf, \"lock \")) {\n-\t\t\tconst char *name = buf.buf + 5;\n+\t\tif ((name = skip_prefix(buf.buf, \"lock \")) != NULL) {\n \t\t\tif (transport->pack_lockfile)\n \t\t\t\twarning(\"%s also locked %s\", data->name, name);\n \t\t\telse\n@@ -643,16 +643,15 @@ static int push_update_ref_status(struct strbuf *buf,\n \t\t\t\t struct ref **ref,\n \t\t\t\t struct ref *remote_refs)\n {\n-\tchar *refname, *msg;\n+\tconst char *refname;\n+\tchar *msg;\n \tint status;\n \n-\tif (starts_with(buf->buf, \"ok \")) {\n+\tif ((refname = skip_prefix(buf->buf, \"ok \")) != NULL)\n \t\tstatus = REF_STATUS_OK;\n-\t\trefname = buf->buf + 3;\n-\t} else if (starts_with(buf->buf, \"error \")) {\n+\telse if ((refname = skip_prefix(buf->buf, \"error \")) != NULL)\n \t\tstatus = REF_STATUS_REMOTE_REJECT;\n-\t\trefname = buf->buf + 6;\n-\t} else\n+\telse\n \t\tdie(\"expected ok/error, helper said '%s'\", buf->buf);\n \n \tmsg = strchr(refname, ' ');\ndiff --git a/transport.c b/transport.c\nindex 824c5b9..e88c2dc 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -147,9 +147,9 @@ static void set_upstreams(struct transport *transport, struct ref *refs,\n {\n \tstruct ref *ref;\n \tfor (ref = refs; ref; ref = ref->next) {\n-\t\tconst char *localname;\n+\t\tconst char *localname, *short_local;\n \t\tconst char *tmp;\n-\t\tconst char *remotename;\n+\t\tconst char *remotename, *short_remote;\n \t\tunsigned char sha[20];\n \t\tint flag = 0;\n \t\t/*\n@@ -173,18 +173,20 @@ static void set_upstreams(struct transport *transport, struct ref *refs,\n \t\t\tlocalname = tmp;\n \n \t\t/* Both source and destination must be local branches. */\n-\t\tif (!localname || !starts_with(localname, \"refs/heads/\"))\n+\t\tif (!localname ||\n+\t\t (short_local = skip_prefix(localname, \"refs/heads/\")) == NULL)\n \t\t\tcontinue;\n-\t\tif (!remotename || !starts_with(remotename, \"refs/heads/\"))\n+\t\tif (!remotename ||\n+\t\t (short_remote = skip_prefix(remotename, \"refs/heads/\")) == NULL)\n \t\t\tcontinue;\n \n \t\tif (!pretend)\n \t\t\tinstall_branch_config(BRANCH_CONFIG_VERBOSE,\n-\t\t\t\tlocalname + 11, transport->remote->name,\n+\t\t\t\tshort_local, transport->remote->name,\n \t\t\t\tremotename);\n \t\telse\n \t\t\tprintf(\"Would set upstream of '%s' to '%s' of '%s'\\n\",\n-\t\t\t\tlocalname + 11, remotename + 11,\n+\t\t\t\tshort_local, short_remote,\n \t\t\t\ttransport->remote->name);\n \t}\n }\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232157","messageId":"1387378437-20646-7-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 06/12] fetch.c: replace some use of starts_with() with skip_prefix()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:51Z","receivedAt":"2013-12-18T14:53:51Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Variable \"what\" is set in an if/else chain. If all fails (and \"what\"\nis set to NULL) it'll be reset in the final \"else\" block.\n\nin get_remote_group(), \"key\" is only used once. So changing it does\nnot harm.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n builtin/fetch.c | 16 +++++-----------\n 1 file changed, 5 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 1e7d617..38f4f7b 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -589,18 +589,12 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\t\t\tkind = \"\";\n \t\t\t\twhat = \"\";\n \t\t\t}\n-\t\t\telse if (starts_with(rm->name, \"refs/heads/\")) {\n+\t\t\telse if ((what = skip_prefix(rm->name, \"refs/heads/\")) != NULL)\n \t\t\t\tkind = \"branch\";\n-\t\t\t\twhat = rm->name + 11;\n-\t\t\t}\n-\t\t\telse if (starts_with(rm->name, \"refs/tags/\")) {\n+\t\t\telse if ((what = skip_prefix(rm->name, \"refs/tags/\")) != NULL)\n \t\t\t\tkind = \"tag\";\n-\t\t\t\twhat = rm->name + 10;\n-\t\t\t}\n-\t\t\telse if (starts_with(rm->name, \"refs/remotes/\")) {\n+\t\t\telse if ((what = skip_prefix(rm->name, \"refs/remotes/\")) != NULL)\n \t\t\t\tkind = \"remote-tracking branch\";\n-\t\t\t\twhat = rm->name + 13;\n-\t\t\t}\n \t\t\telse {\n \t\t\t\tkind = \"\";\n \t\t\t\twhat = rm->name;\n@@ -896,8 +890,8 @@ static int get_remote_group(const char *key, const char *value, void *priv)\n {\n \tstruct remote_group_data *g = priv;\n \n-\tif (starts_with(key, \"remotes.\") &&\n-\t\t\t!strcmp(key + 8, g->name)) {\n+\tif ((key = skip_prefix(key, \"remotes.\")) != NULL &&\n+\t !strcmp(key, g->name)) {\n \t\t/* split list by white space */\n \t\tint space = strcspn(value, \" \\t\\n\");\n \t\twhile (*value) {\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232159","messageId":"1387378437-20646-8-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 07/12] connect.c: replace some use of starts_with() with skip_prefix()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:52Z","receivedAt":"2013-12-18T14:53:52Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\"name\" will be reset unconditionally soon after skip_prefix() returns\nNULL.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n connect.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/connect.c b/connect.c\nindex c763eed..1bb70aa 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -131,7 +131,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \tfor (;;) {\n \t\tstruct ref *ref;\n \t\tunsigned char old_sha1[20];\n-\t\tchar *name;\n+\t\tconst char *name;\n \t\tint len, name_len;\n \t\tchar *buffer = packet_buffer;\n \n@@ -145,8 +145,8 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \t\tif (!len)\n \t\t\tbreak;\n \n-\t\tif (len > 4 && starts_with(buffer, \"ERR \"))\n-\t\t\tdie(\"remote error: %s\", buffer + 4);\n+\t\tif ((name = skip_prefix(buffer, \"ERR \")) != NULL)\n+\t\t\tdie(\"remote error: %s\", name);\n \n \t\tif (len < 42 || get_sha1_hex(buffer, old_sha1) || buffer[40] != ' ')\n \t\t\tdie(\"protocol error: expected sha/ref, got '%s'\", buffer);\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232160","messageId":"1387378437-20646-9-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 08/12] refs.c: replace some use of starts_with() with skip_prefix()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:53Z","receivedAt":"2013-12-18T14:53:53Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"It's out of context, but there are neither changes in buffer nor buf\nbetween two chunks.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 5e378bc..1fb658f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1339,7 +1339,7 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \tfor (;;) {\n \t\tchar path[PATH_MAX];\n \t\tstruct stat st;\n-\t\tchar *buf;\n+\t\tconst char *buf;\n \t\tint fd;\n \n \t\tif (--depth < 0)\n@@ -1415,7 +1415,7 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \t\t/*\n \t\t * Is it a symbolic ref?\n \t\t */\n-\t\tif (!starts_with(buffer, \"ref:\")) {\n+\t\tif ((buf = skip_prefix(buffer, \"ref:\")) == NULL) {\n \t\t\t/*\n \t\t\t * Please note that FETCH_HEAD has a second\n \t\t\t * line containing other data.\n@@ -1430,7 +1430,6 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \t\t}\n \t\tif (flag)\n \t\t\t*flag |= REF_ISSYMREF;\n-\t\tbuf = buffer + 4;\n \t\twhile (isspace(*buf))\n \t\t\tbuf++;\n \t\tif (check_refname_format(buf, REFNAME_ALLOW_ONELEVEL)) {\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232162","messageId":"1387378437-20646-10-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 09/12] diff.c: reduce code duplication in --stat-xxx parsing","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:54Z","receivedAt":"2013-12-18T14:53:54Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n diff.c | 67 +++++++++++++++++++++++++-----------------------------------------\n 1 file changed, 25 insertions(+), 42 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex d754e2f..4da77fd 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3405,6 +3405,23 @@ int parse_long_opt(const char *opt, const char **argv,\n \treturn 2;\n }\n \n+static int parse_statopt(const char *arg, const char *next,\n+\t\t\t const char *opt, int *value, char **end)\n+{\n+\tconst char *p = skip_prefix(arg, opt);\n+\tif (!p)\n+\t\treturn 0;\n+\tif (*p == '=')\n+\t\t*value = strtoul(p + 1, end, 10);\n+\telse if (!*p && !next)\n+\t\tdie(\"Option '--stat%s' requires a value\", opt);\n+\telse if (!*p) {\n+\t\t*value = strtoul(next, end, 10);\n+\t\treturn 2;\n+\t}\n+\treturn 1;\n+}\n+\n static int stat_opt(struct diff_options *options, const char **av)\n {\n \tconst char *arg = av[0];\n@@ -3417,50 +3434,16 @@ static int stat_opt(struct diff_options *options, const char **av)\n \n \targ += strlen(\"--stat\");\n \tend = (char *)arg;\n-\n \tswitch (*arg) {\n \tcase '-':\n-\t\tif (starts_with(arg, \"-width\")) {\n-\t\t\targ += strlen(\"-width\");\n-\t\t\tif (*arg == '=')\n-\t\t\t\twidth = strtoul(arg + 1, &end, 10);\n-\t\t\telse if (!*arg && !av[1])\n-\t\t\t\tdie(\"Option '--stat-width' requires a value\");\n-\t\t\telse if (!*arg) {\n-\t\t\t\twidth = strtoul(av[1], &end, 10);\n-\t\t\t\targcount = 2;\n-\t\t\t}\n-\t\t} else if (starts_with(arg, \"-name-width\")) {\n-\t\t\targ += strlen(\"-name-width\");\n-\t\t\tif (*arg == '=')\n-\t\t\t\tname_width = strtoul(arg + 1, &end, 10);\n-\t\t\telse if (!*arg && !av[1])\n-\t\t\t\tdie(\"Option '--stat-name-width' requires a value\");\n-\t\t\telse if (!*arg) {\n-\t\t\t\tname_width = strtoul(av[1], &end, 10);\n-\t\t\t\targcount = 2;\n-\t\t\t}\n-\t\t} else if (starts_with(arg, \"-graph-width\")) {\n-\t\t\targ += strlen(\"-graph-width\");\n-\t\t\tif (*arg == '=')\n-\t\t\t\tgraph_width = strtoul(arg + 1, &end, 10);\n-\t\t\telse if (!*arg && !av[1])\n-\t\t\t\tdie(\"Option '--stat-graph-width' requires a value\");\n-\t\t\telse if (!*arg) {\n-\t\t\t\tgraph_width = strtoul(av[1], &end, 10);\n-\t\t\t\targcount = 2;\n-\t\t\t}\n-\t\t} else if (starts_with(arg, \"-count\")) {\n-\t\t\targ += strlen(\"-count\");\n-\t\t\tif (*arg == '=')\n-\t\t\t\tcount = strtoul(arg + 1, &end, 10);\n-\t\t\telse if (!*arg && !av[1])\n-\t\t\t\tdie(\"Option '--stat-count' requires a value\");\n-\t\t\telse if (!*arg) {\n-\t\t\t\tcount = strtoul(av[1], &end, 10);\n-\t\t\t\targcount = 2;\n-\t\t\t}\n-\t\t}\n+\t\tif ((argcount = parse_statopt(arg, av[1], \"-width\", &width, &end)) != 0 ||\n+\t\t (argcount = parse_statopt(arg, av[1], \"-name-width\", &name_width, &end)) != 0 ||\n+\t\t (argcount = parse_statopt(arg, av[1], \"-graph-width\", &graph_width, &end)) != 0 ||\n+\t\t (argcount = parse_statopt(arg, av[1], \"-count\", &count, &end)) != 0)\n+\t\t\t/* nothing else, it's the OR chain that's important */\n+\t\t\t;\n+\t\telse\n+\t\t\targcount = 1;\n \t\tbreak;\n \tcase '=':\n \t\twidth = strtoul(arg+1, &end, 10);\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232161","messageId":"1387378437-20646-11-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 10/12] environment.c: replace starts_with() in strip_namespace() with skip_prefix()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:55Z","receivedAt":"2013-12-18T14:53:55Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"Signed-off-by: Nguyễn Thái Ngọc Duy \n---\n environment.c | 4 +---\n 1 file changed, 1 insertion(+), 3 deletions(-)\n\ndiff --git a/environment.c b/environment.c\nindex 3c76905..bc2d916 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -171,9 +171,7 @@ const char *get_git_namespace(void)\n \n const char *strip_namespace(const char *namespaced_ref)\n {\n-\tif (!starts_with(namespaced_ref, get_git_namespace()))\n-\t\treturn NULL;\n-\treturn namespaced_ref + namespace_len;\n+\treturn skip_prefix(namespaced_ref, get_git_namespace());\n }\n \n static int git_work_tree_initialized;\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232164","messageId":"1387378437-20646-12-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 11/12] diff.c: convert diff_scoreopt_parse to use skip_prefix()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:56Z","receivedAt":"2013-12-18T14:53:56Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"While at there, partly fix the reporting as well. The reported value\nwith \"arg+2\" is only correct with -C/-B/-M, not with long option names.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n diff.c | 28 ++++++++++++++--------------\n 1 file changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 4da77fd..d629cc5 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3367,7 +3367,7 @@ static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *va\n \treturn 1;\n }\n \n-static int diff_scoreopt_parse(const char *opt);\n+static int diff_scoreopt_parse(const char **opt);\n \n static inline int short_opt(char opt, const char **argv,\n \t\t\t const char **optarg)\n@@ -3627,13 +3627,13 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t/* renames options */\n \telse if (starts_with(arg, \"-B\") || starts_with(arg, \"--break-rewrites=\") ||\n \t\t !strcmp(arg, \"--break-rewrites\")) {\n-\t\tif ((options->break_opt = diff_scoreopt_parse(arg)) == -1)\n-\t\t\treturn error(\"invalid argument to -B: %s\", arg+2);\n+\t\tif ((options->break_opt = diff_scoreopt_parse(&arg)) == -1)\n+\t\t\treturn error(\"invalid argument to -B: %s\", arg);\n \t}\n \telse if (starts_with(arg, \"-M\") || starts_with(arg, \"--find-renames=\") ||\n \t\t !strcmp(arg, \"--find-renames\")) {\n-\t\tif ((options->rename_score = diff_scoreopt_parse(arg)) == -1)\n-\t\t\treturn error(\"invalid argument to -M: %s\", arg+2);\n+\t\tif ((options->rename_score = diff_scoreopt_parse(&arg)) == -1)\n+\t\t\treturn error(\"invalid argument to -M: %s\", arg);\n \t\toptions->detect_rename = DIFF_DETECT_RENAME;\n \t}\n \telse if (!strcmp(arg, \"-D\") || !strcmp(arg, \"--irreversible-delete\")) {\n@@ -3643,8 +3643,8 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)\n \t\t !strcmp(arg, \"--find-copies\")) {\n \t\tif (options->detect_rename == DIFF_DETECT_COPY)\n \t\t\tDIFF_OPT_SET(options, FIND_COPIES_HARDER);\n-\t\tif ((options->rename_score = diff_scoreopt_parse(arg)) == -1)\n-\t\t\treturn error(\"invalid argument to -C: %s\", arg+2);\n+\t\tif ((options->rename_score = diff_scoreopt_parse(&arg)) == -1)\n+\t\t\treturn error(\"invalid argument to -C: %s\", arg);\n \t\toptions->detect_rename = DIFF_DETECT_COPY;\n \t}\n \telse if (!strcmp(arg, \"--no-renames\"))\n@@ -3879,29 +3879,29 @@ int parse_rename_score(const char **cp_p)\n \treturn (int)((num >= scale) ? MAX_SCORE : (MAX_SCORE * num / scale));\n }\n \n-static int diff_scoreopt_parse(const char *opt)\n+static int diff_scoreopt_parse(const char **opt_p)\n {\n+\tconst char *opt = *opt_p;\n \tint opt1, opt2, cmd;\n \n \tif (*opt++ != '-')\n \t\treturn -1;\n \tcmd = *opt++;\n+\t*opt_p = opt;\n \tif (cmd == '-') {\n \t\t/* convert the long-form arguments into short-form versions */\n-\t\tif (starts_with(opt, \"break-rewrites\")) {\n-\t\t\topt += strlen(\"break-rewrites\");\n+\t\tif ((opt = skip_prefix_defval(opt, \"break-rewrites\", *opt_p)) != *opt_p) {\n \t\t\tif (*opt == 0 || *opt++ == '=')\n \t\t\t\tcmd = 'B';\n-\t\t} else if (starts_with(opt, \"find-copies\")) {\n-\t\t\topt += strlen(\"find-copies\");\n+\t\t} else if ((opt = skip_prefix_defval(opt, \"find-copies\", *opt_p)) != *opt_p) {\n \t\t\tif (*opt == 0 || *opt++ == '=')\n \t\t\t\tcmd = 'C';\n-\t\t} else if (starts_with(opt, \"find-renames\")) {\n-\t\t\topt += strlen(\"find-renames\");\n+\t\t} else if ((opt = skip_prefix_defval(opt, \"find-renames\", *opt_p)) != *opt_p) {\n \t\t\tif (*opt == 0 || *opt++ == '=')\n \t\t\t\tcmd = 'M';\n \t\t}\n \t}\n+\t*opt_p = opt;\n \tif (cmd != 'M' && cmd != 'C' && cmd != 'B')\n \t\treturn -1; /* that is not a -M, -C nor -B option */\n \n-- \n1.8.5.1.208.g019362e\n"},{"id":"232163","messageId":"1387378437-20646-13-git-send-email-pclouds@gmail.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"[PATCH 12/12] refs.c: use skip_prefix() in prune_ref()","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-18T14:53:57Z","receivedAt":"2013-12-18T14:53:57Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"This removes the magic number 5, which is the string length of\n\"refs/\". This comes from get_loose_refs(), called in packed_refs().\n\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n refs.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/refs.c b/refs.c\nindex 1fb658f..217093f 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2318,7 +2318,8 @@ static void try_remove_empty_parents(char *name)\n /* make sure nobody touched the ref, and unlink */\n static void prune_ref(struct ref_to_prune *r)\n {\n-\tstruct ref_lock *lock = lock_ref_sha1(r->name + 5, r->sha1);\n+\tconst char *name = skip_prefix_defval(r->name, \"refs/\", r->name);\n+\tstruct ref_lock *lock = lock_ref_sha1(name, r->sha1);\n \n \tif (lock) {\n \t\tunlink_or_warn(git_path(\"%s\", r->name));\n-- \n1.8.5.1.208.g019362e\n"},{"id":"232169","messageId":"20131218162746.GA15334@sl4mmy.zerosphere.org","threadId":"35546","inReplyTo":"1387378437-20646-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 03/12] Add and use skip_prefix_defval()","fromName":"Kent R. Spillner","fromEmail":"kspillner@acm.org","sentAt":"2013-12-18T16:27:46Z","receivedAt":"2013-12-18T16:27:46Z","isPatch":true,"sender":{"key":"kspillner@acm.org","avatar":"https://gravatar.com/avatar/050ad1f9bbca4ec2de093b251f225080467f6d8180227caf25b591bd123d140a?d=mp&s=160"},"body":"On Wed, Dec 18, 2013 at 09:53:48PM +0700, Nguyễn Thái Ngọc Duy wrote:\n> This is a variant of skip_prefix() that returns a specied pointer\n> instead of NULL if no prefix is found. It's helpful to simplify\n> \n> if (starts_with(foo, \"bar\"))\n> foo += 3;\n> \n> into\n> \n> foo = skip_prefix_gently(foo, \"bar\", foo);\n\nShould this be skip_prefix_defval instead of skip_prefix_gently?\n"},{"id":"232176","messageId":"xmqqa9fyjbg0.fsf@gitster.dls.corp.google.com","threadId":"35546","inReplyTo":"1387378437-20646-2-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 01/12] Make starts_with() a wrapper of skip_prefix()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-18T17:50:07Z","receivedAt":"2013-12-18T17:50:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> starts_with() started out as a copy of prefixcmp(). But if we don't\n> care about the sorting order, the logic looks closer to\n> skip_prefix(). This looks like a good thing to do.\n>\n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n\nSure, but the implementation of skip_prefix() scans the prefix\nstring twice, while prefixcmp() aka starts_with() does it only once.\n\nI'd expect a later step in this series would rectify this micro\nregression in the performance, though ;-)\n\n> git-compat-util.h | 6 +++++-\n> strbuf.c | 9 ---------\n> 2 files changed, 5 insertions(+), 10 deletions(-)\n>\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index b73916b..84f1078 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -350,7 +350,6 @@ extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_lis\n> extern void set_error_routine(void (*routine)(const char *err, va_list params));\n> extern void set_die_is_recursing_routine(int (*routine)(void));\n> \n> -extern int starts_with(const char *str, const char *prefix);\n> extern int prefixcmp(const char *str, const char *prefix);\n> extern int ends_with(const char *str, const char *suffix);\n> extern int suffixcmp(const char *str, const char *suffix);\n> @@ -361,6 +360,11 @@ static inline const char *skip_prefix(const char *str, const char *prefix)\n> \treturn strncmp(str, prefix, len) ? NULL : str + len;\n> }\n> \n> +static inline int starts_with(const char *str, const char *prefix)\n> +{\n> +\treturn skip_prefix(str, prefix) != NULL;\n> +}\n> +\n> #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n> \n> #ifndef PROT_READ\n> diff --git a/strbuf.c b/strbuf.c\n> index 83caf4a..bd4c0d8 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -1,15 +1,6 @@\n> #include \"cache.h\"\n> #include \"refs.h\"\n> \n> -int starts_with(const char *str, const char *prefix)\n> -{\n> -\tfor (; ; str++, prefix++)\n> -\t\tif (!*prefix)\n> -\t\t\treturn 1;\n> -\t\telse if (*str != *prefix)\n> -\t\t\treturn 0;\n> -}\n> -\n> int prefixcmp(const char *str, const char *prefix)\n> {\n> \tfor (; ; str++, prefix++)\n"},{"id":"232177","messageId":"xmqq38lqjbde.fsf@gitster.dls.corp.google.com","threadId":"35546","inReplyTo":"20131218162746.GA15334@sl4mmy.zerosphere.org","subject":"Re: [PATCH 03/12] Add and use skip_prefix_defval()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-18T17:51:40Z","receivedAt":"2013-12-18T17:51:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kent R. Spillner\" writes:\n\n> On Wed, Dec 18, 2013 at 09:53:48PM +0700, Nguyễn Thái Ngọc Duy wrote:\n>> This is a variant of skip_prefix() that returns a specied pointer\n>> instead of NULL if no prefix is found. It's helpful to simplify\n>> \n>> if (starts_with(foo, \"bar\"))\n>> foo += 3;\n>> \n>> into\n>> \n>> foo = skip_prefix_gently(foo, \"bar\", foo);\n>\n> Should this be skip_prefix_defval instead of skip_prefix_gently?\n\nYes.\n"},{"id":"232179","messageId":"xmqqtxe6hw4t.fsf@gitster.dls.corp.google.com","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 00/12] Hard coded string length cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-18T18:06:10Z","receivedAt":"2013-12-18T18:06:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy writes:\n\n> I reimplemented skip_prefix() again just to realize this function\n> already exists. Which reminds me there are a bunch of places that\n> could benefit from this function, the same reason that I wanted to\n> reimplement it.\n>\n> So this is series to make it more popular (so hopefully I'll see it\n> used somewhere and know that it exists) and the code cleaner. The\n> pattern \"compare a string, then skip the compared part by a hard coded\n> string length\" is almost killed. I left a few in places for those who\n> want to contribute :)\n\nOverall the goal of getting rid of the \"if the first N bytes of the\nptr match this string, advance ptr by N bytes\" pattern, whether the\nfirst N is implicitly given by using prefixcmp() or explicitly given\nby using memcmp(), is very good. And the result looked good from a\ncursory read---though I have not gone through the series with fine\ntoothed comb yet.\n\nThanks.\n\n>\n> Nguyễn Thái Ngọc Duy (12):\n> Make starts_with() a wrapper of skip_prefix()\n> Convert starts_with() to skip_prefix() for option parsing\n> Add and use skip_prefix_defval()\n> Replace some use of starts_with() with skip_prefix()\n> Convert a lot of starts_with() to skip_prefix()\n> fetch.c: replace some use of starts_with() with skip_prefix()\n> connect.c: replace some use of starts_with() with skip_prefix()\n> refs.c: replace some use of starts_with() with skip_prefix()\n> diff.c: reduce code duplication in --stat-xxx parsing\n> environment.c: replace starts_with() in strip_namespace() with skip_prefix()\n> diff.c: convert diff_scoreopt_parse to use skip_prefix()\n> refs.c: use skip_prefix() in prune_ref()\n>\n> builtin/branch.c | 3 +-\n> builtin/checkout.c | 6 +-\n> builtin/fast-export.c | 3 +-\n> builtin/fetch-pack.c | 13 ++--\n> builtin/fetch.c | 16 ++---\n> builtin/for-each-ref.c | 9 +--\n> builtin/index-pack.c | 17 +++---\n> builtin/ls-remote.c | 9 +--\n> builtin/mailinfo.c | 11 ++--\n> builtin/merge.c | 12 ++--\n> builtin/reflog.c | 9 +--\n> builtin/remote.c | 3 +-\n> builtin/rev-parse.c | 41 ++++++-------\n> builtin/send-pack.c | 18 +++---\n> builtin/show-branch.c | 14 ++---\n> builtin/unpack-objects.c | 5 +-\n> builtin/update-ref.c | 21 +++----\n> commit.c | 5 +-\n> connect.c | 6 +-\n> daemon.c | 75 +++++++++++------------\n> diff.c | 153 +++++++++++++++++++++--------------------------\n> environment.c | 4 +-\n> fetch-pack.c | 9 +--\n> git-compat-util.h | 15 ++++-\n> git.c | 16 ++---\n> http-backend.c | 5 +-\n> http-push.c | 6 +-\n> http.c | 5 +-\n> log-tree.c | 5 +-\n> merge-recursive.c | 13 ++--\n> notes.c | 6 +-\n> pager.c | 2 +-\n> pathspec.c | 5 +-\n> pretty.c | 3 +-\n> refs.c | 20 ++++---\n> revision.c | 60 +++++++++----------\n> setup.c | 3 +-\n> sha1_name.c | 12 +---\n> strbuf.c | 9 ---\n> tag.c | 7 +--\n> transport-helper.c | 15 +++--\n> transport.c | 14 +++--\n> upload-pack.c | 5 +-\n> wt-status.c | 15 ++---\n> 44 files changed, 334 insertions(+), 369 deletions(-)\n"},{"id":"232180","messageId":"xmqqppouhvny.fsf@gitster.dls.corp.google.com","threadId":"35546","inReplyTo":"xmqqa9fyjbg0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 01/12] Make starts_with() a wrapper of skip_prefix()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-18T18:16:17Z","receivedAt":"2013-12-18T18:16:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Nguyễn Thái Ngọc Duy writes:\n>\n>> starts_with() started out as a copy of prefixcmp(). But if we don't\n>> care about the sorting order, the logic looks closer to\n>> skip_prefix(). This looks like a good thing to do.\n>>\n>> Signed-off-by: Nguyễn Thái Ngọc Duy \n>> ---\n>\n> Sure, but the implementation of skip_prefix() scans the prefix\n> string twice, while prefixcmp() aka starts_with() does it only once.\n>\n> I'd expect a later step in this series would rectify this micro\n> regression in the performance, though ;-)\n\n... and I did not see one, but it would be trivial on top.\n\n git-compat-util.h | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex b72a80d..59265af 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -356,8 +356,11 @@ extern int suffixcmp(const char *str, const char *suffix);\n \n static inline const char *skip_prefix_defval(const char *str, const char *prefix, const char *defval)\n {\n-\tsize_t len = strlen(prefix);\n-\treturn strncmp(str, prefix, len) ? defval : str + len;\n+\tfor ( ; ; str++, prefix++)\n+\t\tif (!*prefix)\n+\t\t\treturn str;\n+\t\telse if (*str != *prefix)\n+\t\t\treturn defval;\n }\n \n static inline const char *skip_prefix(const char *str, const char *prefix)\n"},{"id":"232269","messageId":"52B38213.2070702@web.de","threadId":"35546","inReplyTo":"1387378437-20646-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 00/12] Hard coded string length cleanup","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2013-12-19T23:32:35Z","receivedAt":"2013-12-19T23:32:35Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 18.12.2013 15:53, schrieb Nguyễn Thái Ngọc Duy:\n> I reimplemented skip_prefix() again just to realize this function\n> already exists. Which reminds me there are a bunch of places that\n> could benefit from this function, the same reason that I wanted to\n> reimplement it.\n> \n> So this is series to make it more popular (so hopefully I'll see it\n> used somewhere and know that it exists) and the code cleaner. The\n> pattern \"compare a string, then skip the compared part by a hard coded\n> string length\" is almost killed. I left a few in places for those who\n> want to contribute :)\n\nGood idea.\n\nSeeing that skip_prefix_defval is mostly used in the form\nskip_prefix_defval(foo, prefix, foo) I wonder if it makes sense to\nfirst change skip_prefix to return the full string instead of NULL\nif the prefix is not matched. Would the resulting function cover\nmost use cases? And would it still be easily usable?\n\n---\n advice.c | 2 ++\n builtin/branch.c | 6 +++---\n builtin/clone.c | 6 ++++--\n builtin/commit.c | 6 ++----\n builtin/fmt-merge-msg.c | 6 +++---\n builtin/push.c | 2 --\n builtin/remote.c | 13 +++----------\n column.c | 2 +-\n config.c | 2 +-\n credential-cache--daemon.c | 4 ++--\n credential.c | 2 +-\n git-compat-util.h | 7 +------\n parse-options.c | 11 ++++++-----\n strbuf.c | 10 ++++++++++\n transport.c | 6 +++++-\n urlmatch.c | 2 +-\n 16 files changed, 45 insertions(+), 42 deletions(-)\n\ndiff --git a/advice.c b/advice.c\nindex 3eca9f5..1f85338 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -66,6 +66,8 @@ int git_default_advice_config(const char *var, const char *value)\n \tconst char *k = skip_prefix(var, \"advice.\");\n \tint i;\n \n+\tif (k == var)\n+\t\treturn 0;\n \tfor (i = 0; i < ARRAY_SIZE(advice_config); i++) {\n \t\tif (strcmp(k, advice_config[i].name))\n \t\t\tcontinue;\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex b4d7716..d3694d0 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -294,13 +294,13 @@ static char *resolve_symref(const char *src, const char *prefix)\n {\n \tunsigned char sha1[20];\n \tint flag;\n-\tconst char *dst, *cp;\n+\tconst char *dst;\n \n \tdst = resolve_ref_unsafe(src, sha1, 0, &flag);\n \tif (!(dst && (flag & REF_ISSYMREF)))\n \t\treturn NULL;\n-\tif (prefix && (cp = skip_prefix(dst, prefix)))\n-\t\tdst = cp;\n+\tif (prefix)\n+\t\tdst = skip_prefix(dst, prefix);\n \treturn xstrdup(dst);\n }\n \ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex f98f529..79f24cd 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -578,11 +578,13 @@ static void update_remote_refs(const struct ref *refs,\n static void update_head(const struct ref *our, const struct ref *remote,\n \t\t\tconst char *msg)\n {\n-\tif (our && starts_with(our->name, \"refs/heads/\")) {\n+\tconst char *head;\n+\n+\tif (our &&\n+\t ((head = skip_prefix(our->name, \"refs/heads/\")) != our->name)) {\n \t\t/* Local default branch link */\n \t\tcreate_symref(\"HEAD\", our->name, NULL);\n \t\tif (!option_bare) {\n-\t\t\tconst char *head = skip_prefix(our->name, \"refs/heads/\");\n \t\t\tupdate_ref(msg, \"HEAD\", our->old_sha1, NULL, 0, DIE_ON_ERR);\n \t\t\tinstall_branch_config(0, head, option_origin, our->name);\n \t\t}\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 3767478..c18a77d 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -934,7 +934,7 @@ static int message_is_empty(struct strbuf *sb)\n static int template_untouched(struct strbuf *sb)\n {\n \tstruct strbuf tmpl = STRBUF_INIT;\n-\tchar *start;\n+\tconst char *start;\n \n \tif (cleanup_mode == CLEANUP_NONE && sb->len)\n \t\treturn 0;\n@@ -943,9 +943,7 @@ static int template_untouched(struct strbuf *sb)\n \t\treturn 0;\n \n \tstripspace(&tmpl, cleanup_mode == CLEANUP_ALL);\n-\tstart = (char *)skip_prefix(sb->buf, tmpl.buf);\n-\tif (!start)\n-\t\tstart = sb->buf;\n+\tstart = skip_prefix(sb->buf, tmpl.buf);\n \tstrbuf_release(&tmpl);\n \treturn rest_is_empty(sb, start - sb->buf);\n }\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 3906eda..ff34c62 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -284,7 +284,7 @@ static void credit_people(struct strbuf *out,\n \t\t\t int kind)\n {\n \tconst char *label;\n-\tconst char *me;\n+\tconst char *me, *p;\n \n \tif (kind == 'a') {\n \t\tlabel = \"By\";\n@@ -297,8 +297,8 @@ static void credit_people(struct strbuf *out,\n \tif (!them->nr ||\n \t (them->nr == 1 &&\n \t me &&\n-\t (me = skip_prefix(me, them->items->string)) != NULL &&\n-\t skip_prefix(me, \" <\")))\n+\t (p = skip_prefix(me, them->items->string)) != me &&\n+\t starts_with(p, \" <\")))\n \t\treturn;\n \tstrbuf_addf(out, \"\\n%c %s \", comment_line_char, label);\n \tadd_people_count(out, them);\ndiff --git a/builtin/push.c b/builtin/push.c\nindex a73982a..2852a46 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -91,8 +91,6 @@ static NORETURN int die_push_simple(struct branch *branch, struct remote *remote\n \tconst char *short_upstream =\n \t\tskip_prefix(branch->merge[0]->src, \"refs/heads/\");\n \n-\tif (!short_upstream)\n-\t\tshort_upstream = branch->merge[0]->src;\n \t/*\n \t * Don't show advice for people who explicitly set\n \t * push.default.\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex b3ab4cf..1f5dfbe 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -248,14 +248,7 @@ struct branch_info {\n \n static struct string_list branch_list;\n \n-static const char *abbrev_ref(const char *name, const char *prefix)\n-{\n-\tconst char *abbrev = skip_prefix(name, prefix);\n-\tif (abbrev)\n-\t\treturn abbrev;\n-\treturn name;\n-}\n-#define abbrev_branch(name) abbrev_ref((name), \"refs/heads/\")\n+#define abbrev_branch(name) skip_prefix((name), \"refs/heads/\")\n \n static int config_read_branches(const char *key, const char *value, void *cb)\n {\n@@ -1326,10 +1319,10 @@ static int prune_remote(const char *remote, int dry_run)\n \n \t\tif (dry_run)\n \t\t\tprintf_ln(_(\" * [would prune] %s\"),\n-\t\t\t abbrev_ref(refname, \"refs/remotes/\"));\n+\t\t\t skip_prefix(refname, \"refs/remotes/\"));\n \t\telse\n \t\t\tprintf_ln(_(\" * [pruned] %s\"),\n-\t\t\t abbrev_ref(refname, \"refs/remotes/\"));\n+\t\t\t skip_prefix(refname, \"refs/remotes/\"));\n \t\twarn_dangling_symref(stdout, dangling_msg, refname);\n \t}\n \ndiff --git a/column.c b/column.c\nindex 9367ba5..7de051d 100644\n--- a/column.c\n+++ b/column.c\n@@ -337,7 +337,7 @@ int git_column_config(const char *var, const char *value,\n \t\t const char *command, unsigned int *colopts)\n {\n \tconst char *it = skip_prefix(var, \"column.\");\n-\tif (!it)\n+\tif (it == var)\n \t\treturn 0;\n \n \tif (!strcmp(it, \"ui\"))\ndiff --git a/config.c b/config.c\nindex d969a5a..b787f8d 100644\n--- a/config.c\n+++ b/config.c\n@@ -134,7 +134,7 @@ int git_config_include(const char *var, const char *value, void *data)\n \t\treturn ret;\n \n \ttype = skip_prefix(var, \"include.\");\n-\tif (!type)\n+\tif (type == var)\n \t\treturn ret;\n \n \tif (!strcmp(type, \"path\"))\ndiff --git a/credential-cache--daemon.c b/credential-cache--daemon.c\nindex 390f194..21aad75 100644\n--- a/credential-cache--daemon.c\n+++ b/credential-cache--daemon.c\n@@ -110,13 +110,13 @@ static int read_request(FILE *fh, struct credential *c,\n \n \tstrbuf_getline(&item, fh, '\\n');\n \tp = skip_prefix(item.buf, \"action=\");\n-\tif (!p)\n+\tif (p == item.buf)\n \t\treturn error(\"client sent bogus action line: %s\", item.buf);\n \tstrbuf_addstr(action, p);\n \n \tstrbuf_getline(&item, fh, '\\n');\n \tp = skip_prefix(item.buf, \"timeout=\");\n-\tif (!p)\n+\tif (p == item.buf)\n \t\treturn error(\"client sent bogus timeout line: %s\", item.buf);\n \t*timeout = atoi(p);\n \ndiff --git a/credential.c b/credential.c\nindex e54753c..466beff 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -41,7 +41,7 @@ static int credential_config_callback(const char *var, const char *value,\n \tconst char *key, *dot;\n \n \tkey = skip_prefix(var, \"credential.\");\n-\tif (!key)\n+\tif (key == var)\n \t\treturn 0;\n \n \tif (!value)\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex b73916b..dcb92c4 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -354,12 +354,7 @@ extern int starts_with(const char *str, const char *prefix);\n extern int prefixcmp(const char *str, const char *prefix);\n extern int ends_with(const char *str, const char *suffix);\n extern int suffixcmp(const char *str, const char *suffix);\n-\n-static inline const char *skip_prefix(const char *str, const char *prefix)\n-{\n-\tsize_t len = strlen(prefix);\n-\treturn strncmp(str, prefix, len) ? NULL : str + len;\n-}\n+extern const char *skip_prefix(const char *str, const char *prefix);\n \n #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n \ndiff --git a/parse-options.c b/parse-options.c\nindex 7b8d3fa..4ec2fa3 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -240,7 +240,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,\n again:\n \t\trest = skip_prefix(arg, long_name);\n \t\tif (options->type == OPTION_ARGUMENT) {\n-\t\t\tif (!rest)\n+\t\t\tif (rest == arg)\n \t\t\t\tcontinue;\n \t\t\tif (*rest == '=')\n \t\t\t\treturn opterror(options, \"takes no value\", flags);\n@@ -249,7 +249,7 @@ again:\n \t\t\tp->out[p->cpidx++] = arg - 2;\n \t\t\treturn 0;\n \t\t}\n-\t\tif (!rest) {\n+\t\tif (rest == arg) {\n \t\t\t/* abbreviated? */\n \t\t\tif (!strncmp(long_name, arg, arg_end - arg)) {\n is_abbreviated:\n@@ -289,10 +289,11 @@ is_abbreviated:\n \t\t\tflags |= OPT_UNSET;\n \t\t\trest = skip_prefix(arg + 3, long_name);\n \t\t\t/* abbreviated and negated? */\n-\t\t\tif (!rest && starts_with(long_name, arg + 3))\n-\t\t\t\tgoto is_abbreviated;\n-\t\t\tif (!rest)\n+\t\t\tif (rest == arg + 3) {\n+\t\t\t\tif (starts_with(long_name, arg + 3))\n+\t\t\t\t\tgoto is_abbreviated;\n \t\t\t\tcontinue;\n+\t\t\t}\n \t\t}\n \t\tif (*rest) {\n \t\t\tif (*rest != '=')\ndiff --git a/strbuf.c b/strbuf.c\nindex 83caf4a..222df13 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -37,6 +37,16 @@ int suffixcmp(const char *str, const char *suffix)\n \t\treturn strcmp(str + len - suflen, suffix);\n }\n \n+const char *skip_prefix(const char *str, const char *prefix)\n+{\n+\tconst char *p;\n+\tfor (p = str; ; p++, prefix++)\n+\t\tif (!*prefix)\n+\t\t\treturn p;\n+\t\telse if (*p != *prefix)\n+\t\t\treturn str;\n+}\n+\n /*\n * Used as the default ->buf value, so that people can always assume\n * buf is non NULL and ->buf is NUL terminated even for a freshly\ndiff --git a/transport.c b/transport.c\nindex 824c5b9..8d3372f 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -191,7 +191,11 @@ static void set_upstreams(struct transport *transport, struct ref *refs,\n \n static const char *rsync_url(const char *url)\n {\n-\treturn !starts_with(url, \"rsync://\") ? skip_prefix(url, \"rsync:\") : url;\n+\tif (!starts_with(url, \"rsync://\")) {\n+\t\tconst char *rest = skip_prefix(url, \"rsync:\");\n+\t\turl = (rest == url) ? NULL : rest;\n+\t}\n+\treturn url;\n }\n \n static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)\ndiff --git a/urlmatch.c b/urlmatch.c\nindex ec87cba..51b5d23 100644\n--- a/urlmatch.c\n+++ b/urlmatch.c\n@@ -484,7 +484,7 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)\n \tint retval;\n \n \tkey = skip_prefix(var, collect->section);\n-\tif (!key || *(key++) != '.') {\n+\tif ((key == var) || (*(key++) != '.')) {\n \t\tif (collect->cascade_fn)\n \t\t\treturn collect->cascade_fn(var, value, cb);\n \t\treturn 0; /* not interested */\n-- \n1.8.5.2\n"},{"id":"232272","messageId":"CACsJy8Bb4+V1DEdEmRwj10Oozi8U430ZHDCj_UhnXZcaR-wQ=g@mail.gmail.com","threadId":"35546","inReplyTo":"52B38213.2070702@web.de","subject":"Re: [PATCH 00/12] Hard coded string length cleanup","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-19T23:50:25Z","receivedAt":"2013-12-19T23:50:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Dec 20, 2013 at 6:32 AM, René Scharfe wrote:\n> Seeing that skip_prefix_defval is mostly used in the form\n> skip_prefix_defval(foo, prefix, foo) I wonder if it makes sense to\n> first change skip_prefix to return the full string instead of NULL\n> if the prefix is not matched. Would the resulting function cover\n> most use cases? And would it still be easily usable?\n\nThat was skip_prefix_gently() that I forgot to replace in a commit\nmessage, before I turned it into _defval variant. The reason for\n_defval is it could be use to chain expression together without adding\ntemporary variables, e.g.\n\n- if (starts_with(line->buf, \">From\") && isspace(line->buf[5])) {\n+ if (isspace(*skip_prefix_defval(line->buf, \">From\", \"NOSPACE\"))) {\n\nWithout _defval, one would need to do if ((p = skip_prefix(..)) &&\nisspace(*p)). I'm not entirely sure this is a good thing though as it\ncould make it a bit harder to read.\n-- \nDuy\n"},{"id":"232275","messageId":"52B397FF.4050808@web.de","threadId":"35546","inReplyTo":"CACsJy8Bb4+V1DEdEmRwj10Oozi8U430ZHDCj_UhnXZcaR-wQ=g@mail.gmail.com","subject":"Re: [PATCH 00/12] Hard coded string length cleanup","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2013-12-20T01:06:07Z","receivedAt":"2013-12-20T01:06:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 20.12.2013 00:50, schrieb Duy Nguyen:\n> On Fri, Dec 20, 2013 at 6:32 AM, René Scharfe wrote:\n>> Seeing that skip_prefix_defval is mostly used in the form\n>> skip_prefix_defval(foo, prefix, foo) I wonder if it makes sense to\n>> first change skip_prefix to return the full string instead of NULL\n>> if the prefix is not matched. Would the resulting function cover\n>> most use cases? And would it still be easily usable?\n> \n> That was skip_prefix_gently() that I forgot to replace in a commit\n> message, before I turned it into _defval variant. The reason for\n> _defval is it could be use to chain expression together without adding\n> temporary variables, e.g.\n> \n> - if (starts_with(line->buf, \">From\") && isspace(line->buf[5])) {\n> + if (isspace(*skip_prefix_defval(line->buf, \">From\", \"NOSPACE\"))) {\n> \n> Without _defval, one would need to do if ((p = skip_prefix(..)) &&\n> isspace(*p)). I'm not entirely sure this is a good thing though as it\n> could make it a bit harder to read.\n\nThat usage is quite rare compared to occurrences of\nskip_prefix_defval(foo, prefix, foo), no? Adding a temporary variable\nfor them wouldn't be that bad if we can simplify the API to a single\nfunction -- if that one is usable, that is.\n\nOn the other hand, we could add a special function for that example\nand we'd already have three users in the tree (patch below). I think\nthat's too narrow a use case for a library function, though. Doing\nthe following instead in the three cases doesn't seem to be too bad:\n\n\trest = skip_prefix(line->buf, \">From\");\n\tif (rest != line->buf && isspace(*rest)) {\n\n---\n builtin/apply.c | 2 +-\n builtin/mailinfo.c | 4 ++--\n git-compat-util.h | 1 +\n strbuf.c | 9 +++++++++\n 4 files changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/apply.c b/builtin/apply.c\nindex b0d0986..b96befd 100644\n--- a/builtin/apply.c\n+++ b/builtin/apply.c\n@@ -433,7 +433,7 @@ static unsigned long linelen(const char *buffer, unsigned long size)\n \n static int is_dev_null(const char *str)\n {\n-\treturn !memcmp(\"/dev/null\", str, 9) && isspace(str[9]);\n+\treturn skip_prefix_and_space(str, \"/dev/null\") != str;\n }\n \n #define TERM_SPACE\t1\ndiff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\nindex 2c3cd8e..2575989 100644\n--- a/builtin/mailinfo.c\n+++ b/builtin/mailinfo.c\n@@ -328,11 +328,11 @@ static int check_header(const struct strbuf *line,\n \t}\n \n \t/* for inbody stuff */\n-\tif (starts_with(line->buf, \">From\") && isspace(line->buf[5])) {\n+\tif (skip_prefix_and_space(line->buf, \">From\") != line->buf) {\n \t\tret = 1; /* Should this return 0? */\n \t\tgoto check_header_out;\n \t}\n-\tif (starts_with(line->buf, \"[PATCH]\") && isspace(line->buf[7])) {\n+\tif (skip_prefix_and_space(line->buf, \"[PATCH]\") != line->buf) {\n \t\tfor (i = 0; header[i]; i++) {\n \t\t\tif (!memcmp(\"Subject\", header[i], 7)) {\n \t\t\t\thandle_header(&hdr_data[i], line);\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex dcb92c4..a083918 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -355,6 +355,7 @@ extern int prefixcmp(const char *str, const char *prefix);\n extern int ends_with(const char *str, const char *suffix);\n extern int suffixcmp(const char *str, const char *suffix);\n extern const char *skip_prefix(const char *str, const char *prefix);\n+extern const char *skip_prefix_and_space(const char *str, const char *prefix);\n \n #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n \ndiff --git a/strbuf.c b/strbuf.c\nindex 222df13..768331f 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -47,6 +47,15 @@ const char *skip_prefix(const char *str, const char *prefix)\n \t\t\treturn str;\n }\n \n+const char *skip_prefix_and_space(const char *str, const char *prefix)\n+{\n+\tconst char *p = skip_prefix(str, prefix);\n+\tif (((p != str) || !*prefix) && isspace(*p))\n+\t\treturn p + 1;\n+\telse\n+\t\treturn str;\n+}\n+\n /*\n * Used as the default ->buf value, so that people can always assume\n * buf is non NULL and ->buf is NUL terminated even for a freshly\n-- \n1.8.5.2\n"},{"id":"232276","messageId":"CACsJy8BqzgC6t_rFJ+Q7MHqEoHjdT8f-XgvRBY1aNjiUvCq=WQ@mail.gmail.com","threadId":"35546","inReplyTo":"52B397FF.4050808@web.de","subject":"Re: [PATCH 00/12] Hard coded string length cleanup","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-20T02:29:32Z","receivedAt":"2013-12-20T02:29:32Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Dec 20, 2013 at 8:06 AM, René Scharfe wrote:\n> Am 20.12.2013 00:50, schrieb Duy Nguyen:\n>> On Fri, Dec 20, 2013 at 6:32 AM, René Scharfe wrote:\n>>> Seeing that skip_prefix_defval is mostly used in the form\n>>> skip_prefix_defval(foo, prefix, foo) I wonder if it makes sense to\n>>> first change skip_prefix to return the full string instead of NULL\n>>> if the prefix is not matched. Would the resulting function cover\n>>> most use cases? And would it still be easily usable?\n>>\n>> That was skip_prefix_gently() that I forgot to replace in a commit\n>> message, before I turned it into _defval variant. The reason for\n>> _defval is it could be use to chain expression together without adding\n>> temporary variables, e.g.\n>>\n>> - if (starts_with(line->buf, \">From\") && isspace(line->buf[5])) {\n>> + if (isspace(*skip_prefix_defval(line->buf, \">From\", \"NOSPACE\"))) {\n>>\n>> Without _defval, one would need to do if ((p = skip_prefix(..)) &&\n>> isspace(*p)). I'm not entirely sure this is a good thing though as it\n>> could make it a bit harder to read.\n>\n> That usage is quite rare compared to occurrences of\n> skip_prefix_defval(foo, prefix, foo), no? Adding a temporary variable\n> for them wouldn't be that bad if we can simplify the API to a single\n> function -- if that one is usable, that is.\n>\n> On the other hand, we could add a special function for that example\n> and we'd already have three users in the tree (patch below). I think\n> that's too narrow a use case for a library function, though. Doing\n> the following instead in the three cases doesn't seem to be too bad:\n>\n> rest = skip_prefix(line->buf, \">From\");\n> if (rest != line->buf && isspace(*rest)) {\n>\n\nOK I agree it's the minority and probably not even worth a wrapper.\nBut I disagree on changing the behavior of skip_prefix(). Some\nin-flight topics might depend on \"old\" skip_prefix(). The\n\"skip_prefix() returns full string if not found\" should have a\ndifferent name. I'll go with _gently again, unless you suggest another\nname.\n\n> ---\n> builtin/apply.c | 2 +-\n> builtin/mailinfo.c | 4 ++--\n> git-compat-util.h | 1 +\n> strbuf.c | 9 +++++++++\n> 4 files changed, 13 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/apply.c b/builtin/apply.c\n> index b0d0986..b96befd 100644\n> --- a/builtin/apply.c\n> +++ b/builtin/apply.c\n> @@ -433,7 +433,7 @@ static unsigned long linelen(const char *buffer, unsigned long size)\n>\n> static int is_dev_null(const char *str)\n> {\n> - return !memcmp(\"/dev/null\", str, 9) && isspace(str[9]);\n> + return skip_prefix_and_space(str, \"/dev/null\") != str;\n> }\n>\n> #define TERM_SPACE 1\n> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\n> index 2c3cd8e..2575989 100644\n> --- a/builtin/mailinfo.c\n> +++ b/builtin/mailinfo.c\n> @@ -328,11 +328,11 @@ static int check_header(const struct strbuf *line,\n> }\n>\n> /* for inbody stuff */\n> - if (starts_with(line->buf, \">From\") && isspace(line->buf[5])) {\n> + if (skip_prefix_and_space(line->buf, \">From\") != line->buf) {\n> ret = 1; /* Should this return 0? */\n> goto check_header_out;\n> }\n> - if (starts_with(line->buf, \"[PATCH]\") && isspace(line->buf[7])) {\n> + if (skip_prefix_and_space(line->buf, \"[PATCH]\") != line->buf) {\n> for (i = 0; header[i]; i++) {\n> if (!memcmp(\"Subject\", header[i], 7)) {\n> handle_header(&hdr_data[i], line);\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index dcb92c4..a083918 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -355,6 +355,7 @@ extern int prefixcmp(const char *str, const char *prefix);\n> extern int ends_with(const char *str, const char *suffix);\n> extern int suffixcmp(const char *str, const char *suffix);\n> extern const char *skip_prefix(const char *str, const char *prefix);\n> +extern const char *skip_prefix_and_space(const char *str, const char *prefix);\n>\n> #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n>\n> diff --git a/strbuf.c b/strbuf.c\n> index 222df13..768331f 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -47,6 +47,15 @@ const char *skip_prefix(const char *str, const char *prefix)\n> return str;\n> }\n>\n> +const char *skip_prefix_and_space(const char *str, const char *prefix)\n> +{\n> + const char *p = skip_prefix(str, prefix);\n> + if (((p != str) || !*prefix) && isspace(*p))\n> + return p + 1;\n> + else\n> + return str;\n> +}\n> +\n> /*\n> * Used as the default ->buf value, so that people can always assume\n> * buf is non NULL and ->buf is NUL terminated even for a freshly\n> --\n> 1.8.5.2\n\n\n\n-- \nDuy\n"},{"id":"232277","messageId":"52B3E8D4.1030805@viscovery.net","threadId":"35546","inReplyTo":"1387378437-20646-3-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2013-12-20T06:51:00Z","receivedAt":"2013-12-20T06:51:00Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12/18/2013 15:53, schrieb Nguyễn Thái Ngọc Duy:\n> The code that's not converted to use parse_options() often does\n> \n> if (!starts_with(arg, \"foo=\")) {\n> value = atoi(arg + 4);\n> }\n> \n> This patch removes those magic numbers with skip_prefix()\n> \n> Signed-off-by: Nguyễn Thái Ngọc Duy \n> ---\n> builtin/fetch-pack.c | 13 +++++----\n> builtin/index-pack.c | 17 +++++------\n> builtin/ls-remote.c | 9 +++---\n> builtin/mailinfo.c | 5 ++--\n> builtin/reflog.c | 9 +++---\n> builtin/rev-parse.c | 41 +++++++++++++-------------\n> builtin/send-pack.c | 18 ++++++------\n> builtin/unpack-objects.c | 5 ++--\n> builtin/update-ref.c | 21 +++++++-------\n> daemon.c | 75 ++++++++++++++++++++++++------------------------\n> diff.c | 49 +++++++++++++++----------------\n> git.c | 13 +++++----\n> merge-recursive.c | 13 +++++----\n> revision.c | 60 +++++++++++++++++++-------------------\n> upload-pack.c | 5 ++--\n> 15 files changed, 182 insertions(+), 171 deletions(-)\n> \n> diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\n> index 8b8978a2..2df1423 100644\n> --- a/builtin/fetch-pack.c\n> +++ b/builtin/fetch-pack.c\n> @@ -47,13 +47,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n> \n> \tfor (i = 1; i < argc && *argv[i] == '-'; i++) {\n> \t\tconst char *arg = argv[i];\n> +\t\tconst char *optarg;\n> \n> -\t\tif (starts_with(arg, \"--upload-pack=\")) {\n> -\t\t\targs.uploadpack = arg + 14;\n> +\t\tif ((optarg = skip_prefix(arg, \"--upload-pack=\")) != NULL) {\n> +\t\t\targs.uploadpack = optarg;\n\nQuite frankly, I do not think this is an improvement. The old code is\n*MUCH* easier to understand because \"starts_with\" is clearly a predicate\nthat is either true or false, but the code with \"skip_prefix\" is much\nheavier on the eye with its extra level of parenthesis. That it removes a\nhard-coded constant does not count much IMHO because it is very clear\nwhere the value comes from.\n\n-- Hannes\n"},{"id":"232278","messageId":"20131220070449.GA29717@sigill.intra.peff.net","threadId":"35546","inReplyTo":"52B3E8D4.1030805@viscovery.net","subject":"Re: [PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-12-20T07:04:49Z","receivedAt":"2013-12-20T07:04:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 20, 2013 at 07:51:00AM +0100, Johannes Sixt wrote:\n\n> > \tfor (i = 1; i < argc && *argv[i] == '-'; i++) {\n> > \t\tconst char *arg = argv[i];\n> > +\t\tconst char *optarg;\n> > \n> > -\t\tif (starts_with(arg, \"--upload-pack=\")) {\n> > -\t\t\targs.uploadpack = arg + 14;\n> > +\t\tif ((optarg = skip_prefix(arg, \"--upload-pack=\")) != NULL) {\n> > +\t\t\targs.uploadpack = optarg;\n> \n> Quite frankly, I do not think this is an improvement. The old code is\n> *MUCH* easier to understand because \"starts_with\" is clearly a predicate\n> that is either true or false, but the code with \"skip_prefix\" is much\n> heavier on the eye with its extra level of parenthesis. That it removes a\n> hard-coded constant does not count much IMHO because it is very clear\n> where the value comes from.\n\nYeah, I agree that is unfortunate. Maybe we could have the best of both\nworlds, like:\n\n if (starts_with(arg, \"--upload-pack=\", &optarg))\n ... use optarg ...\n\nProbably we do not want to call it just \"starts_with\", as quite a few\ncallers to do not care about what comes next, and would just pass NULL.\n\nI cannot seem to think of a good name, though, as the \"with\" means that\nobvious things like \"starts_with_value\" naturally parse as a single\n(nonsensical) sentence. Something like \"parse_prefix\" would work, but\nit is not as clearly a predicate as \"starts_with\" (but we have at least\ngotten rid of the extra parentheses).\n\nElsewhere in the thread, the concept was discussed of returning the full\nstring to mean \"did not match\", which makes some other idioms simpler\n(but IMHO makes the simple cases like this even harder to read). My\nproposal splits the \"start of string\" out parameter from the boolean\nreturn, so it handles both cases naturally:\n\n /* here we care if we saw the prefix, as above */\n if (parse_prefix(foo, prefix, &the_rest))\n ...\n\n /*\n * and here we do not care, and just want to optionally strip the\n * prefix, and take the full value otherwise; we just have to ignore\n * the return value in this case.\n */\n parse_prefix(foo, prefix, &foo);\n\n-Peff\n"},{"id":"232279","messageId":"CAP8UFD1Otpm209EGL0XWS5sCDXev9HqZT2BZ9+Sk285J-sZ-yg@mail.gmail.com","threadId":"35546","inReplyTo":"20131220070449.GA29717@sigill.intra.peff.net","subject":"Re: [PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2013-12-20T08:46:41Z","receivedAt":"2013-12-20T08:46:41Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Dec 20, 2013 at 8:04 AM, Jeff King wrote:\n> On Fri, Dec 20, 2013 at 07:51:00AM +0100, Johannes Sixt wrote:\n>\n>> > for (i = 1; i < argc && *argv[i] == '-'; i++) {\n>> > const char *arg = argv[i];\n>> > + const char *optarg;\n>> >\n>> > - if (starts_with(arg, \"--upload-pack=\")) {\n>> > - args.uploadpack = arg + 14;\n>> > + if ((optarg = skip_prefix(arg, \"--upload-pack=\")) != NULL) {\n>> > + args.uploadpack = optarg;\n>>\n>> Quite frankly, I do not think this is an improvement. The old code is\n>> *MUCH* easier to understand because \"starts_with\" is clearly a predicate\n>> that is either true or false, but the code with \"skip_prefix\" is much\n>> heavier on the eye with its extra level of parenthesis. That it removes a\n>> hard-coded constant does not count much IMHO because it is very clear\n>> where the value comes from.\n>\n> Yeah, I agree that is unfortunate.\n\nI agree too.\n\n> Maybe we could have the best of both\n> worlds, like:\n>\n> if (starts_with(arg, \"--upload-pack=\", &optarg))\n> ... use optarg ...\n>\n> Probably we do not want to call it just \"starts_with\", as quite a few\n> callers to do not care about what comes next, and would just pass NULL.\n> I cannot seem to think of a good name, though, as the \"with\" means that\n> obvious things like \"starts_with_value\" naturally parse as a single\n> (nonsensical) sentence. Something like \"parse_prefix\" would work, but\n> it is not as clearly a predicate as \"starts_with\" (but we have at least\n> gotten rid of the extra parentheses).\n>\n> Elsewhere in the thread, the concept was discussed of returning the full\n> string to mean \"did not match\", which makes some other idioms simpler\n> (but IMHO makes the simple cases like this even harder to read). My\n> proposal splits the \"start of string\" out parameter from the boolean\n> return, so it handles both cases naturally:\n>\n> /* here we care if we saw the prefix, as above */\n> if (parse_prefix(foo, prefix, &the_rest))\n> ...\n>\n> /*\n> * and here we do not care, and just want to optionally strip the\n> * prefix, and take the full value otherwise; we just have to ignore\n> * the return value in this case.\n> */\n> parse_prefix(foo, prefix, &foo);\n\nYeah, I agree that the function signature you suggest is better, but I\nlike the \"skip_prefix\" name better.\nOr perhaps \"remove_prefix\"?\n\nThanks,\nChristian.\n"},{"id":"232283","messageId":"52B41F42.7050204@web.de","threadId":"35546","inReplyTo":"20131220070449.GA29717@sigill.intra.peff.net","subject":"Re: [PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2013-12-20T10:43:14Z","receivedAt":"2013-12-20T10:43:14Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 20.12.2013 08:04, schrieb Jeff King:\n> On Fri, Dec 20, 2013 at 07:51:00AM +0100, Johannes Sixt wrote:\n> \n>>> \tfor (i = 1; i < argc && *argv[i] == '-'; i++) {\n>>> \t\tconst char *arg = argv[i];\n>>> +\t\tconst char *optarg;\n>>> \n>>> -\t\tif (starts_with(arg, \"--upload-pack=\")) {\n>>> -\t\t\targs.uploadpack = arg + 14;\n>>> +\t\tif ((optarg = skip_prefix(arg, \"--upload-pack=\")) != NULL) {\n>>> +\t\t\targs.uploadpack = optarg;\n>>\n>> Quite frankly, I do not think this is an improvement. The old code is\n>> *MUCH* easier to understand because \"starts_with\" is clearly a predicate\n>> that is either true or false, but the code with \"skip_prefix\" is much\n>> heavier on the eye with its extra level of parenthesis. That it removes a\n>> hard-coded constant does not count much IMHO because it is very clear\n>> where the value comes from.\n> \n> Yeah, I agree that is unfortunate. Maybe we could have the best of both\n> worlds, like:\n> \n> if (starts_with(arg, \"--upload-pack=\", &optarg))\n> ... use optarg ...\n> \n> Probably we do not want to call it just \"starts_with\", as quite a few\n> callers to do not care about what comes next, and would just pass NULL.\n> \n> I cannot seem to think of a good name, though, as the \"with\" means that\n> obvious things like \"starts_with_value\" naturally parse as a single\n> (nonsensical) sentence. Something like \"parse_prefix\" would work, but\n> it is not as clearly a predicate as \"starts_with\" (but we have at least\n> gotten rid of the extra parentheses).\n> \n> Elsewhere in the thread, the concept was discussed of returning the full\n> string to mean \"did not match\", which makes some other idioms simpler\n> (but IMHO makes the simple cases like this even harder to read). My\n> proposal splits the \"start of string\" out parameter from the boolean\n> return, so it handles both cases naturally:\n> \n> /* here we care if we saw the prefix, as above */\n> if (parse_prefix(foo, prefix, &the_rest))\n> ...\n> \n> /*\n> * and here we do not care, and just want to optionally strip the\n> * prefix, and take the full value otherwise; we just have to ignore\n> * the return value in this case.\n> */\n> parse_prefix(foo, prefix, &foo);\n\nIt adds a bit of redundancy, but overall I like it. It fits the common\ncase very well and looks nice. The patch below converts all calls of\nskip_prefix as well as the usage of starts_with and a magic number in\nbuiltin/fetch-pack.c.\n\nI wonder how many of the 400+ uses of starts_with remain after a\nparse_prefix crusade. If only a few remain then it may make sense\nto unite the two functions under a common name.\n\n---\n advice.c | 5 ++++-\n builtin/branch.c | 6 +++---\n builtin/clone.c | 13 ++++++++-----\n builtin/commit.c | 6 ++----\n builtin/fetch-pack.c | 14 +++++++-------\n builtin/fmt-merge-msg.c | 4 ++--\n builtin/push.c | 7 +++----\n builtin/remote.c | 4 +---\n column.c | 5 +++--\n config.c | 3 +--\n credential-cache--daemon.c | 6 ++----\n credential.c | 3 +--\n git-compat-util.h | 1 +\n parse-options.c | 11 ++++++-----\n strbuf.c | 12 +++++++++---\n transport.c | 6 +++++-\n urlmatch.c | 3 +--\n 17 files changed, 59 insertions(+), 50 deletions(-)\n\ndiff --git a/advice.c b/advice.c\nindex 3eca9f5..75fae9c 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -63,9 +63,12 @@ void advise(const char *advice, ...)\n \n int git_default_advice_config(const char *var, const char *value)\n {\n-\tconst char *k = skip_prefix(var, \"advice.\");\n+\tconst char *k;\n \tint i;\n \n+\tif (!parse_prefix(var, \"advice.\", &k))\n+\t\treturn 0;\n+\n \tfor (i = 0; i < ARRAY_SIZE(advice_config); i++) {\n \t\tif (strcmp(k, advice_config[i].name))\n \t\t\tcontinue;\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex b4d7716..dae0d82 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -294,13 +294,13 @@ static char *resolve_symref(const char *src, const char *prefix)\n {\n \tunsigned char sha1[20];\n \tint flag;\n-\tconst char *dst, *cp;\n+\tconst char *dst;\n \n \tdst = resolve_ref_unsafe(src, sha1, 0, &flag);\n \tif (!(dst && (flag & REF_ISSYMREF)))\n \t\treturn NULL;\n-\tif (prefix && (cp = skip_prefix(dst, prefix)))\n-\t\tdst = cp;\n+\tif (prefix)\n+\t\tparse_prefix(dst, prefix, &dst);\n \treturn xstrdup(dst);\n }\n \ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex f98f529..e62fa26 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -578,11 +578,12 @@ static void update_remote_refs(const struct ref *refs,\n static void update_head(const struct ref *our, const struct ref *remote,\n \t\t\tconst char *msg)\n {\n-\tif (our && starts_with(our->name, \"refs/heads/\")) {\n+\tconst char *head;\n+\n+\tif (our && parse_prefix(our->name, \"refs/heads/\", &head)) {\n \t\t/* Local default branch link */\n \t\tcreate_symref(\"HEAD\", our->name, NULL);\n \t\tif (!option_bare) {\n-\t\t\tconst char *head = skip_prefix(our->name, \"refs/heads/\");\n \t\t\tupdate_ref(msg, \"HEAD\", our->old_sha1, NULL, 0, DIE_ON_ERR);\n \t\t\tinstall_branch_config(0, head, option_origin, our->name);\n \t\t}\n@@ -696,9 +697,11 @@ static void write_refspec_config(const char* src_ref_prefix,\n \t\t\t\t\tstrbuf_addf(&value, \"+%s:%s%s\", our_head_points_at->name,\n \t\t\t\t\t\tbranch_top->buf, option_branch);\n \t\t\t} else if (remote_head_points_at) {\n-\t\t\t\tstrbuf_addf(&value, \"+%s:%s%s\", remote_head_points_at->name,\n-\t\t\t\t\t\tbranch_top->buf,\n-\t\t\t\t\t\tskip_prefix(remote_head_points_at->name, \"refs/heads/\"));\n+\t\t\t\tconst char *name = remote_head_points_at->name;\n+\t\t\t\tconst char *head = NULL;\n+\t\t\t\tparse_prefix(name, \"refs/heads/\", &head);\n+\t\t\t\tstrbuf_addf(&value, \"+%s:%s%s\",\n+\t\t\t\t\t name, branch_top->buf, head);\n \t\t\t}\n \t\t\t/*\n \t\t\t * otherwise, the next \"git fetch\" will\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 3767478..e9bff59 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -934,7 +934,7 @@ static int message_is_empty(struct strbuf *sb)\n static int template_untouched(struct strbuf *sb)\n {\n \tstruct strbuf tmpl = STRBUF_INIT;\n-\tchar *start;\n+\tconst char *start = sb->buf;\n \n \tif (cleanup_mode == CLEANUP_NONE && sb->len)\n \t\treturn 0;\n@@ -943,9 +943,7 @@ static int template_untouched(struct strbuf *sb)\n \t\treturn 0;\n \n \tstripspace(&tmpl, cleanup_mode == CLEANUP_ALL);\n-\tstart = (char *)skip_prefix(sb->buf, tmpl.buf);\n-\tif (!start)\n-\t\tstart = sb->buf;\n+\tparse_prefix(start, tmpl.buf, &start);\n \tstrbuf_release(&tmpl);\n \treturn rest_is_empty(sb, start - sb->buf);\n }\ndiff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c\nindex 8b8978a2..d673986 100644\n--- a/builtin/fetch-pack.c\n+++ b/builtin/fetch-pack.c\n@@ -46,14 +46,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \targs.uploadpack = \"git-upload-pack\";\n \n \tfor (i = 1; i < argc && *argv[i] == '-'; i++) {\n-\t\tconst char *arg = argv[i];\n+\t\tconst char *optarg, *arg = argv[i];\n \n-\t\tif (starts_with(arg, \"--upload-pack=\")) {\n-\t\t\targs.uploadpack = arg + 14;\n+\t\tif (parse_prefix(arg, \"--upload-pack=\", &optarg)) {\n+\t\t\targs.uploadpack = optarg;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--exec=\")) {\n-\t\t\targs.uploadpack = arg + 7;\n+\t\tif (parse_prefix(arg, \"--exec=\", &optarg)) {\n+\t\t\targs.uploadpack = optarg;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(\"--quiet\", arg) || !strcmp(\"-q\", arg)) {\n@@ -89,8 +89,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)\n \t\t\targs.verbose = 1;\n \t\t\tcontinue;\n \t\t}\n-\t\tif (starts_with(arg, \"--depth=\")) {\n-\t\t\targs.depth = strtol(arg + 8, NULL, 0);\n+\t\tif (parse_prefix(arg, \"--depth=\", &optarg)) {\n+\t\t\targs.depth = strtol(optarg, NULL, 0);\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(\"--no-progress\", arg)) {\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex 3906eda..ce889e3 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -297,8 +297,8 @@ static void credit_people(struct strbuf *out,\n \tif (!them->nr ||\n \t (them->nr == 1 &&\n \t me &&\n-\t (me = skip_prefix(me, them->items->string)) != NULL &&\n-\t skip_prefix(me, \" <\")))\n+\t parse_prefix(me, them->items->string, &me) &&\n+\t starts_with(me, \" <\")))\n \t\treturn;\n \tstrbuf_addf(out, \"\\n%c %s \", comment_line_char, label);\n \tadd_people_count(out, them);\ndiff --git a/builtin/push.c b/builtin/push.c\nindex a73982a..f040e82 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -88,11 +88,10 @@ static NORETURN int die_push_simple(struct branch *branch, struct remote *remote\n \t * them the big ugly fully qualified ref.\n \t */\n \tconst char *advice_maybe = \"\";\n-\tconst char *short_upstream =\n-\t\tskip_prefix(branch->merge[0]->src, \"refs/heads/\");\n+\tconst char *short_upstream = branch->merge[0]->src;\n+\n+\tparse_prefix(short_upstream, \"refs/heads/\", &short_upstream);\n \n-\tif (!short_upstream)\n-\t\tshort_upstream = branch->merge[0]->src;\n \t/*\n \t * Don't show advice for people who explicitly set\n \t * push.default.\ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex b3ab4cf..30d1987 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -250,9 +250,7 @@ static struct string_list branch_list;\n \n static const char *abbrev_ref(const char *name, const char *prefix)\n {\n-\tconst char *abbrev = skip_prefix(name, prefix);\n-\tif (abbrev)\n-\t\treturn abbrev;\n+\tparse_prefix(name, prefix, &name);\n \treturn name;\n }\n #define abbrev_branch(name) abbrev_ref((name), \"refs/heads/\")\ndiff --git a/column.c b/column.c\nindex 9367ba5..dfb2392 100644\n--- a/column.c\n+++ b/column.c\n@@ -336,8 +336,9 @@ static int column_config(const char *var, const char *value,\n int git_column_config(const char *var, const char *value,\n \t\t const char *command, unsigned int *colopts)\n {\n-\tconst char *it = skip_prefix(var, \"column.\");\n-\tif (!it)\n+\tconst char *it;\n+\n+\tif (!parse_prefix(var, \"column.\", &it))\n \t\treturn 0;\n \n \tif (!strcmp(it, \"ui\"))\ndiff --git a/config.c b/config.c\nindex d969a5a..ee42010 100644\n--- a/config.c\n+++ b/config.c\n@@ -133,8 +133,7 @@ int git_config_include(const char *var, const char *value, void *data)\n \tif (ret < 0)\n \t\treturn ret;\n \n-\ttype = skip_prefix(var, \"include.\");\n-\tif (!type)\n+\tif (!parse_prefix(var, \"include.\", &type))\n \t\treturn ret;\n \n \tif (!strcmp(type, \"path\"))\ndiff --git a/credential-cache--daemon.c b/credential-cache--daemon.c\nindex 390f194..3823a38 100644\n--- a/credential-cache--daemon.c\n+++ b/credential-cache--daemon.c\n@@ -109,14 +109,12 @@ static int read_request(FILE *fh, struct credential *c,\n \tconst char *p;\n \n \tstrbuf_getline(&item, fh, '\\n');\n-\tp = skip_prefix(item.buf, \"action=\");\n-\tif (!p)\n+\tif (!parse_prefix(item.buf, \"action=\", &p))\n \t\treturn error(\"client sent bogus action line: %s\", item.buf);\n \tstrbuf_addstr(action, p);\n \n \tstrbuf_getline(&item, fh, '\\n');\n-\tp = skip_prefix(item.buf, \"timeout=\");\n-\tif (!p)\n+\tif (!parse_prefix(item.buf, \"timeout=\", &p))\n \t\treturn error(\"client sent bogus timeout line: %s\", item.buf);\n \t*timeout = atoi(p);\n \ndiff --git a/credential.c b/credential.c\nindex e54753c..fd148a0 100644\n--- a/credential.c\n+++ b/credential.c\n@@ -40,8 +40,7 @@ static int credential_config_callback(const char *var, const char *value,\n \tstruct credential *c = data;\n \tconst char *key, *dot;\n \n-\tkey = skip_prefix(var, \"credential.\");\n-\tif (!key)\n+\tif (!parse_prefix(var, \"credential.\", &key))\n \t\treturn 0;\n \n \tif (!value)\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex b73916b..0a8354a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -350,6 +350,7 @@ extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_lis\n extern void set_error_routine(void (*routine)(const char *err, va_list params));\n extern void set_die_is_recursing_routine(int (*routine)(void));\n \n+extern int parse_prefix(const char *str, const char *prefix, const char **rest);\n extern int starts_with(const char *str, const char *prefix);\n extern int prefixcmp(const char *str, const char *prefix);\n extern int ends_with(const char *str, const char *suffix);\ndiff --git a/parse-options.c b/parse-options.c\nindex 7b8d3fa..1e25203 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -238,7 +238,8 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,\n \t\t\tcontinue;\n \n again:\n-\t\trest = skip_prefix(arg, long_name);\n+\t\trest = NULL;\n+\t\tparse_prefix(arg, long_name, &rest);\n \t\tif (options->type == OPTION_ARGUMENT) {\n \t\t\tif (!rest)\n \t\t\t\tcontinue;\n@@ -287,12 +288,12 @@ is_abbreviated:\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tflags |= OPT_UNSET;\n-\t\t\trest = skip_prefix(arg + 3, long_name);\n \t\t\t/* abbreviated and negated? */\n-\t\t\tif (!rest && starts_with(long_name, arg + 3))\n-\t\t\t\tgoto is_abbreviated;\n-\t\t\tif (!rest)\n+\t\t\tif (!parse_prefix(arg + 3, long_name, &rest)) {\n+\t\t\t\tif (starts_with(long_name, arg + 3))\n+\t\t\t\t\tgoto is_abbreviated;\n \t\t\t\tcontinue;\n+\t\t\t}\n \t\t}\n \t\tif (*rest) {\n \t\t\tif (*rest != '=')\ndiff --git a/strbuf.c b/strbuf.c\nindex 83caf4a..b78bc44 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1,15 +1,21 @@\n #include \"cache.h\"\n #include \"refs.h\"\n \n-int starts_with(const char *str, const char *prefix)\n+int parse_prefix(const char *str, const char *prefix, const char **rest)\n {\n \tfor (; ; str++, prefix++)\n-\t\tif (!*prefix)\n+\t\tif (!*prefix) {\n+\t\t\t*rest = str;\n \t\t\treturn 1;\n-\t\telse if (*str != *prefix)\n+\t\t} else if (*str != *prefix)\n \t\t\treturn 0;\n }\n \n+int starts_with(const char *str, const char *prefix)\n+{\n+\treturn parse_prefix(str, prefix, &str);\n+}\n+\n int prefixcmp(const char *str, const char *prefix)\n {\n \tfor (; ; str++, prefix++)\ndiff --git a/transport.c b/transport.c\nindex 824c5b9..775f2b1 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -191,7 +191,11 @@ static void set_upstreams(struct transport *transport, struct ref *refs,\n \n static const char *rsync_url(const char *url)\n {\n-\treturn !starts_with(url, \"rsync://\") ? skip_prefix(url, \"rsync:\") : url;\n+\tconst char *rest = NULL;\n+\tif (starts_with(url, \"rsync://\"))\n+\t\treturn url;\n+\tparse_prefix(url, \"rsync:\", &rest);\n+\treturn rest;\n }\n \n static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)\ndiff --git a/urlmatch.c b/urlmatch.c\nindex ec87cba..dfd1fa7 100644\n--- a/urlmatch.c\n+++ b/urlmatch.c\n@@ -483,8 +483,7 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)\n \tint user_matched = 0;\n \tint retval;\n \n-\tkey = skip_prefix(var, collect->section);\n-\tif (!key || *(key++) != '.') {\n+\tif (!parse_prefix(var, collect->section, &key) || *(key++) != '.') {\n \t\tif (collect->cascade_fn)\n \t\t\treturn collect->cascade_fn(var, value, cb);\n \t\treturn 0; /* not interested */\n-- \n1.8.5.2\n"},{"id":"232288","messageId":"xmqqfvpnea60.fsf@gitster.dls.corp.google.com","threadId":"35546","inReplyTo":"52B397FF.4050808@web.de","subject":"Re: [PATCH 00/12] Hard coded string length cleanup","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-20T16:53:27Z","receivedAt":"2013-12-20T16:53:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe writes:\n\n> Am 20.12.2013 00:50, schrieb Duy Nguyen:\n>> On Fri, Dec 20, 2013 at 6:32 AM, René Scharfe wrote:\n>>> Seeing that skip_prefix_defval is mostly used in the form\n>>> skip_prefix_defval(foo, prefix, foo) I wonder if it makes sense to\n>>> first change skip_prefix to return the full string instead of NULL\n>>> if the prefix is not matched. Would the resulting function cover\n>>> most use cases? And would it still be easily usable?\n>> \n>> That was skip_prefix_gently() that I forgot to replace in a commit\n>> message, before I turned it into _defval variant. The reason for\n>> _defval is it could be use to chain expression together without adding\n>> temporary variables, e.g.\n>> \n>> - if (starts_with(line->buf, \">From\") && isspace(line->buf[5])) {\n>> + if (isspace(*skip_prefix_defval(line->buf, \">From\", \"NOSPACE\"))) {\n>> \n>> Without _defval, one would need to do if ((p = skip_prefix(..)) &&\n>> isspace(*p)). I'm not entirely sure this is a good thing though as it\n>> could make it a bit harder to read.\n>\n> That usage is quite rare compared to occurrences of\n> skip_prefix_defval(foo, prefix, foo), no? Adding a temporary variable\n> for them wouldn't be that bad if we can simplify the API to a single\n> function -- if that one is usable, that is.\n>\n> On the other hand, we could add a special function for that example\n> and we'd already have three users in the tree (patch below). I think\n> that's too narrow a use case for a library function, though. Doing\n> the following instead in the three cases doesn't seem to be too bad:\n>\n> \trest = skip_prefix(line->buf, \">From\");\n> \tif (rest != line->buf && isspace(*rest)) {\n\nYeah, I personally feel that the \"NOSPACE\" hack is a bit too ugly to\nlive in a code meant to be maintained for a longer term. The above\nwith a \"rest\" variable, whose assignment is outside if () condition,\nis so far the easiest to read, at least to me.\n\nI am not convinced if skip-prefix-and-space is even a good\nabstraction of anything; it feels a bit too specialized.\n\nThanks.\n\n> ---\n> builtin/apply.c | 2 +-\n> builtin/mailinfo.c | 4 ++--\n> git-compat-util.h | 1 +\n> strbuf.c | 9 +++++++++\n> 4 files changed, 13 insertions(+), 3 deletions(-)\n>\n> diff --git a/builtin/apply.c b/builtin/apply.c\n> index b0d0986..b96befd 100644\n> --- a/builtin/apply.c\n> +++ b/builtin/apply.c\n> @@ -433,7 +433,7 @@ static unsigned long linelen(const char *buffer, unsigned long size)\n> \n> static int is_dev_null(const char *str)\n> {\n> -\treturn !memcmp(\"/dev/null\", str, 9) && isspace(str[9]);\n> +\treturn skip_prefix_and_space(str, \"/dev/null\") != str;\n> }\n> \n> #define TERM_SPACE\t1\n> diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\n> index 2c3cd8e..2575989 100644\n> --- a/builtin/mailinfo.c\n> +++ b/builtin/mailinfo.c\n> @@ -328,11 +328,11 @@ static int check_header(const struct strbuf *line,\n> \t}\n> \n> \t/* for inbody stuff */\n> -\tif (starts_with(line->buf, \">From\") && isspace(line->buf[5])) {\n> +\tif (skip_prefix_and_space(line->buf, \">From\") != line->buf) {\n> \t\tret = 1; /* Should this return 0? */\n> \t\tgoto check_header_out;\n> \t}\n> -\tif (starts_with(line->buf, \"[PATCH]\") && isspace(line->buf[7])) {\n> +\tif (skip_prefix_and_space(line->buf, \"[PATCH]\") != line->buf) {\n> \t\tfor (i = 0; header[i]; i++) {\n> \t\t\tif (!memcmp(\"Subject\", header[i], 7)) {\n> \t\t\t\thandle_header(&hdr_data[i], line);\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index dcb92c4..a083918 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -355,6 +355,7 @@ extern int prefixcmp(const char *str, const char *prefix);\n> extern int ends_with(const char *str, const char *suffix);\n> extern int suffixcmp(const char *str, const char *suffix);\n> extern const char *skip_prefix(const char *str, const char *prefix);\n> +extern const char *skip_prefix_and_space(const char *str, const char *prefix);\n> \n> #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)\n> \n> diff --git a/strbuf.c b/strbuf.c\n> index 222df13..768331f 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -47,6 +47,15 @@ const char *skip_prefix(const char *str, const char *prefix)\n> \t\t\treturn str;\n> }\n> \n> +const char *skip_prefix_and_space(const char *str, const char *prefix)\n> +{\n> +\tconst char *p = skip_prefix(str, prefix);\n> +\tif (((p != str) || !*prefix) && isspace(*p))\n> +\t\treturn p + 1;\n> +\telse\n> +\t\treturn str;\n> +}\n> +\n> /*\n> * Used as the default ->buf value, so that people can always assume\n> * buf is non NULL and ->buf is NUL terminated even for a freshly\n"},{"id":"232294","messageId":"xmqq38lndxae.fsf@gitster.dls.corp.google.com","threadId":"35546","inReplyTo":"20131220070449.GA29717@sigill.intra.peff.net","subject":"Re: [PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-20T21:31:37Z","receivedAt":"2013-12-20T21:31:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> /* here we care if we saw the prefix, as above */\n> if (parse_prefix(foo, prefix, &the_rest))\n> ...\n>\n> /*\n> * and here we do not care, and just want to optionally strip the\n> * prefix, and take the full value otherwise; we just have to ignore\n> * the return value in this case.\n> */\n> parse_prefix(foo, prefix, &foo);\n\nSounds fine. I recall earlier somebody wanting to have a good name\nfor this thing, and I think foo_gently is *not* it (the name is\nabout adding a variant that does not die outright to foo that checks\nand dies if condition is not right). \n\n\tstarts_with(foo, prefix);\n strip_prefix(foo, prefix, &foo);\n\nperhaps?\n"},{"id":"232297","messageId":"CACsJy8A0TAyBoTdyrRDb54j9uoZ0=cOuxgTfFavRiUOpHTV_ag@mail.gmail.com","threadId":"35546","inReplyTo":"xmqq38lndxae.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-12-21T04:44:08Z","receivedAt":"2013-12-21T04:44:08Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sat, Dec 21, 2013 at 4:31 AM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>> /* here we care if we saw the prefix, as above */\n>> if (parse_prefix(foo, prefix, &the_rest))\n>> ...\n>>\n>> /*\n>> * and here we do not care, and just want to optionally strip the\n>> * prefix, and take the full value otherwise; we just have to ignore\n>> * the return value in this case.\n>> */\n>> parse_prefix(foo, prefix, &foo);\n>\n> Sounds fine. I recall earlier somebody wanting to have a good name\n> for this thing, and I think foo_gently is *not* it (the name is\n> about adding a variant that does not die outright to foo that checks\n> and dies if condition is not right).\n>\n> starts_with(foo, prefix);\n> strip_prefix(foo, prefix, &foo);\n>\n> perhaps?\n\nI still need consensus on the name here guys, parse_prefix.\nremove_prefix or strip_prefix? If no other opinions i'll go with\nstrip_prefix (Jeff's comment before parse_prefix() also uses \"strip\")\n-- \nDuy\n"},{"id":"232413","messageId":"xmqqha9vct0x.fsf@gitster.dls.corp.google.com","threadId":"35546","inReplyTo":"CACsJy8A0TAyBoTdyrRDb54j9uoZ0=cOuxgTfFavRiUOpHTV_ag@mail.gmail.com","subject":"Re: [PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-12-26T19:27:10Z","receivedAt":"2013-12-26T19:27:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen writes:\n\n> On Sat, Dec 21, 2013 at 4:31 AM, Junio C Hamano wrote:\n>> Jeff King writes:\n>>\n>>> /* here we care if we saw the prefix, as above */\n>>> if (parse_prefix(foo, prefix, &the_rest))\n>>> ...\n>>>\n>>> /*\n>>> * and here we do not care, and just want to optionally strip the\n>>> * prefix, and take the full value otherwise; we just have to ignore\n>>> * the return value in this case.\n>>> */\n>>> parse_prefix(foo, prefix, &foo);\n>>\n>> Sounds fine. I recall earlier somebody wanting to have a good name\n>> for this thing, and I think foo_gently is *not* it (the name is\n>> about adding a variant that does not die outright to foo that checks\n>> and dies if condition is not right).\n>>\n>> starts_with(foo, prefix);\n>> strip_prefix(foo, prefix, &foo);\n>>\n>> perhaps?\n>\n> I still need consensus on the name here guys, parse_prefix.\n> remove_prefix or strip_prefix? If no other opinions i'll go with\n> strip_prefix (Jeff's comment before parse_prefix() also uses \"strip\")\n\nYup, that comment is where I took \"strip\" from. When you name your\nthing as \"X\", using too generic a word \"X\", and then need to explain\nwhat \"X\" does using a bit more specific word \"Y\", you are often\nbetter off naming it after \"Y\".\n"},{"id":"232470","messageId":"20131228095440.GA24166@sigill.intra.peff.net","threadId":"35546","inReplyTo":"xmqqha9vct0x.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 02/12] Convert starts_with() to skip_prefix() for option parsing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-12-28T09:54:40Z","receivedAt":"2013-12-28T09:54:40Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 26, 2013 at 11:27:10AM -0800, Junio C Hamano wrote:\n\n> > I still need consensus on the name here guys, parse_prefix.\n> > remove_prefix or strip_prefix? If no other opinions i'll go with\n> > strip_prefix (Jeff's comment before parse_prefix() also uses \"strip\")\n> \n> Yup, that comment is where I took \"strip\" from. When you name your\n> thing as \"X\", using too generic a word \"X\", and then need to explain\n> what \"X\" does using a bit more specific word \"Y\", you are often\n> better off naming it after \"Y\".\n\nFWIW, the reason I shied away from \"strip\" is that I did not want to\nimply that the function mutates the string. But since nobody else seems\nconcerned with that, I think \"strip\" is fine.\n\n-Peff\n"}]}