{"thread":{"id":"36075","subject":"[PATCH v3] commit.c: Replace starts_with() with skip_prefix()","startedAt":"2014-03-06T17:05:21Z","lastAt":"2014-03-09T11:10:42Z","messageCount":7,"participants":["Karthik Nayak","Junio C Hamano","Eric Sunshine","karthik nayak"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"236169","messageId":"1394125521-9341-1-git-send-email-karthik.188@gmail.com","threadId":"36075","inReplyTo":null,"subject":"[PATCH v3] commit.c: Replace starts_with() with skip_prefix()","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2014-03-06T17:05:21Z","receivedAt":"2014-03-06T17:05:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Replace all instances of starts_with() by skip_prefix(),\nwhich can not only be used to check presence of a prefix,\nbut also used further on as it returns the string after the prefix,\nif the prefix is present. And also manages to do, what the current\ncode does in two steps.\n\nSigned-off-by: Karthik Nayak \n---\nHello Eric,\nIn this patch, I have:\n1. Fixed the improper placement of buf_date , initialised to a NULL string.\n2. Fixed whitespace.\n3. Better naming as per your suggestion.\n4. Fixed the initilisation before the if statement.\nThanks for your suggestion.\n---\n commit.c | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 6bf4fe0..4144e00 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -553,6 +553,7 @@ static void record_author_date(struct author_date_slab *author_date,\n \tstruct ident_split ident;\n \tchar *date_end;\n \tunsigned long date;\n+\tconst char *buf_date;\n \n \tif (!commit->buffer) {\n \t\tunsigned long size;\n@@ -565,15 +566,15 @@ static void record_author_date(struct author_date_slab *author_date,\n \tfor (buf = commit->buffer ? commit->buffer : buffer;\n \t buf;\n \t buf = line_end + 1) {\n+\t\tbuf_date = skip_prefix(buf, \"author \");\n \t\tline_end = strchrnul(buf, '\\n');\n-\t\tif (!starts_with(buf, \"author \")) {\n+\t\tif (!buf_date) {\n \t\t\tif (!line_end[0] || line_end[1] == '\\n')\n \t\t\t\treturn; /* end of header */\n \t\t\tcontinue;\n \t\t}\n-\t\tif (split_ident_line(&ident,\n-\t\t\t\t buf + strlen(\"author \"),\n-\t\t\t\t line_end - (buf + strlen(\"author \"))) ||\n+\t\tif (split_ident_line(&ident, buf_date,\n+\t\t\t\t line_end - buf_date) ||\n \t\t !ident.date_begin || !ident.date_end)\n \t\t\tgoto fail_exit; /* malformed \"author\" line */\n \t\tbreak;\n@@ -1098,6 +1099,7 @@ int parse_signed_commit(const unsigned char *sha1,\n \tchar *buffer = read_sha1_file(sha1, &type, &size);\n \tint in_signature, saw_signature = -1;\n \tchar *line, *tail;\n+\tconst char *gpg_sig;\n \n \tif (!buffer || type != OBJ_COMMIT)\n \t\tgoto cleanup;\n@@ -1113,9 +1115,9 @@ int parse_signed_commit(const unsigned char *sha1,\n \t\tnext = next ? next + 1 : tail;\n \t\tif (in_signature && line[0] == ' ')\n \t\t\tsig = line + 1;\n-\t\telse if (starts_with(line, gpg_sig_header) &&\n-\t\t\t line[gpg_sig_header_len] == ' ')\n-\t\t\tsig = line + gpg_sig_header_len + 1;\n+\t\telse if ((gpg_sig = skip_prefix(line, gpg_sig_header))\n+\t\t\t && gpg_sig[0] == ' ')\n+\t\t\tsig = gpg_sig + 1;\n \t\tif (sig) {\n \t\t\tstrbuf_add(signature, sig, next - sig);\n \t\t\tsaw_signature = 1;\n@@ -1193,10 +1195,8 @@ 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\tfound = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);\n+\t\tif (!found) {\n \t\t\tfound = strstr(buf, sigcheck_gpg_status[i].check);\n \t\t\tif (!found)\n \t\t\t\tcontinue;\n-- \n1.9.0.138.g2de3478\n"},{"id":"236187","messageId":"xmqqlhwnxgo7.fsf@gitster.dls.corp.google.com","threadId":"36075","inReplyTo":"1394125521-9341-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v3] commit.c: Replace starts_with() with skip_prefix()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-06T19:36:56Z","receivedAt":"2014-03-06T19:36:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"We already have 147972b1 (commit.c: use skip_prefix() instead of\nstarts_with(), 2014-03-04) that covers the record_author_date() and\nparse_gpg_output(), don't we?\n"},{"id":"236188","messageId":"xmqqfvmvxgf5.fsf@gitster.dls.corp.google.com","threadId":"36075","inReplyTo":"1394125521-9341-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v3] commit.c: Replace starts_with() with skip_prefix()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-06T19:42:22Z","receivedAt":"2014-03-06T19:42:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> @@ -1098,6 +1099,7 @@ int parse_signed_commit(const unsigned char *sha1,\n> \tchar *buffer = read_sha1_file(sha1, &type, &size);\n> \tint in_signature, saw_signature = -1;\n> \tchar *line, *tail;\n> +\tconst char *gpg_sig;\n> \n> \tif (!buffer || type != OBJ_COMMIT)\n> \t\tgoto cleanup;\n> @@ -1113,9 +1115,9 @@ int parse_signed_commit(const unsigned char *sha1,\n> \t\tnext = next ? next + 1 : tail;\n> \t\tif (in_signature && line[0] == ' ')\n> \t\t\tsig = line + 1;\n> -\t\telse if (starts_with(line, gpg_sig_header) &&\n> -\t\t\t line[gpg_sig_header_len] == ' ')\n> -\t\t\tsig = line + gpg_sig_header_len + 1;\n> +\t\telse if ((gpg_sig = skip_prefix(line, gpg_sig_header))\n> +\t\t\t && gpg_sig[0] == ' ')\n> +\t\t\tsig = gpg_sig + 1;\n\nI am not sure if this hunk is a great improvement, as we know the\nlength of what we are skipping in the gpg_sig_header_len constant\nthat is used throughout this file.\n"},{"id":"236256","messageId":"CAPig+cR0tw4em_mPNLdp1-gNER2G1FyAVCyZAb9_TbtSyuGyaA@mail.gmail.com","threadId":"36075","inReplyTo":"1394125521-9341-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH v3] commit.c: Replace starts_with() with skip_prefix()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-07T09:34:22Z","receivedAt":"2014-03-07T09:34:22Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Mar 6, 2014 at 12:05 PM, Karthik Nayak wrote:\n> Replace all instances of starts_with() by skip_prefix(),\n> which can not only be used to check presence of a prefix,\n> but also used further on as it returns the string after the prefix,\n> if the prefix is present. And also manages to do, what the current\n> code does in two steps.\n\nBetter. Thanks.\n\n> Signed-off-by: Karthik Nayak \n> ---\n> Hello Eric,\n> In this patch, I have:\n> 1. Fixed the improper placement of buf_date , initialised to a NULL string.\n> 2. Fixed whitespace.\n> 3. Better naming as per your suggestion.\n> 4. Fixed the initilisation before the if statement.\n> Thanks for your suggestion.\n> ---\n> commit.c | 22 +++++++++++-----------\n> 1 file changed, 11 insertions(+), 11 deletions(-)\n>\n> diff --git a/commit.c b/commit.c\n> index 6bf4fe0..4144e00 100644\n> --- a/commit.c\n> +++ b/commit.c\n> @@ -553,6 +553,7 @@ static void record_author_date(struct author_date_slab *author_date,\n> struct ident_split ident;\n> char *date_end;\n> unsigned long date;\n> + const char *buf_date;\n>\n> if (!commit->buffer) {\n> unsigned long size;\n> @@ -565,15 +566,15 @@ static void record_author_date(struct author_date_slab *author_date,\n> for (buf = commit->buffer ? commit->buffer : buffer;\n> buf;\n> buf = line_end + 1) {\n> + buf_date = skip_prefix(buf, \"author \");\n\nThe data after \"author \" is identification information (name, email),\nnot date. In fact, this information gets passed to the function\nsplit_ident_line(), so a better name for this variable is 'ident_line'\n(but not the misspelling 'indent_line' from one of your earlier\nattempts).\n\n> line_end = strchrnul(buf, '\\n');\n> - if (!starts_with(buf, \"author \")) {\n> + if (!buf_date) {\n> if (!line_end[0] || line_end[1] == '\\n')\n> return; /* end of header */\n> continue;\n> }\n> - if (split_ident_line(&ident,\n> - buf + strlen(\"author \"),\n> - line_end - (buf + strlen(\"author \"))) ||\n> + if (split_ident_line(&ident, buf_date,\n> + line_end - buf_date) ||\n> !ident.date_begin || !ident.date_end)\n> goto fail_exit; /* malformed \"author\" line */\n> break;\n> @@ -1098,6 +1099,7 @@ int parse_signed_commit(const unsigned char *sha1,\n> char *buffer = read_sha1_file(sha1, &type, &size);\n> int in_signature, saw_signature = -1;\n> char *line, *tail;\n> + const char *gpg_sig;\n>\n> if (!buffer || type != OBJ_COMMIT)\n> goto cleanup;\n> @@ -1113,9 +1115,9 @@ int parse_signed_commit(const unsigned char *sha1,\n> next = next ? next + 1 : tail;\n> if (in_signature && line[0] == ' ')\n> sig = line + 1;\n> - else if (starts_with(line, gpg_sig_header) &&\n> - line[gpg_sig_header_len] == ' ')\n> - sig = line + gpg_sig_header_len + 1;\n> + else if ((gpg_sig = skip_prefix(line, gpg_sig_header))\n> + && gpg_sig[0] == ' ')\n> + sig = gpg_sig + 1;\n\nOther than the poor variable name 'buf_date' and the review comments\nby Junio and Tanay that this particular change is of questionable\nvalue and perhaps should be dropped, this version of the patch looks\nreasonable.\n\nThanks.\n\n> if (sig) {\n> strbuf_add(signature, sig, next - sig);\n> saw_signature = 1;\n> @@ -1193,10 +1195,8 @@ static void parse_gpg_output(struct signature_check *sigc)\n> for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {\n> const char *found, *next;\n>\n> - if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {\n> - /* At the very beginning of the buffer */\n> - found = buf + strlen(sigcheck_gpg_status[i].check + 1);\n> - } else {\n> + found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);\n> + if (!found) {\n> found = strstr(buf, sigcheck_gpg_status[i].check);\n> if (!found)\n> continue;\n> --\n> 1.9.0.138.g2de3478\n>\n"},{"id":"236259","messageId":"CAOLa=ZQZOxEzXUEwCVpesNzRhJ5_sMbtbUpaYjdfEOOrakbG0Q@mail.gmail.com","threadId":"36075","inReplyTo":"CAPig+cR0tw4em_mPNLdp1-gNER2G1FyAVCyZAb9_TbtSyuGyaA@mail.gmail.com","subject":"Re: [PATCH v3] commit.c: Replace starts_with() with skip_prefix()","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2014-03-07T12:39:19Z","receivedAt":"2014-03-07T12:39:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Hello Eric,\nThanks for your reply, and for that information. should i patch again\nor this should do?\nAnd what next? Talk to the mentor?\nThanks\n"},{"id":"236334","messageId":"CAPig+cS9Eh7bde90SsVWqeWP+fZqDTV3fk6FryoP5SFg-FyfTw@mail.gmail.com","threadId":"36075","inReplyTo":"CAOLa=ZSjEv+p17--8UPN7=p+DfLWyEftjAXZDnxuaDKo96Lj7w@mail.gmail.com","subject":"Re: [PATCH v3] commit.c: Replace starts_with() with skip_prefix()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-09T07:49:53Z","receivedAt":"2014-03-09T07:49:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Mar 7, 2014 at 5:49 AM, karthik nayak wrote:\n> Hello Eric,\n> Thanks for your reply, and for that information. should i patch again or\n> this should do?\n> And what next? Talk to the mentor?\n\nThe ultimate authority deciding if a patch is ready is Junio, as it\nwould have to be accepted into his tree. Since he already accepted a\nsimilar patch from a different potential GSoC applicant, it may not\nmake sense to refine this one further. What is important is that you\nare now familiar with the review process on this project, and the\nmentors (hopefully) have gained insight into your abilities and how\nyou interact with reviewers (which was the goal of these\nmicroprojects).\n\nProbably best at this point is to consider a proposed project [1] or\nchoose your own, and start the task of applying for a GSoC position\n(by whatever means that is done).\n\n[1]: https://github.com/git/git.github.io/blob/master/SoC-2014-Ideas.md\n\n> On Fri, Mar 7, 2014 at 3:04 PM, Eric Sunshine \n> wrote:\n>>\n>> On Thu, Mar 6, 2014 at 12:05 PM, Karthik Nayak \n>> wrote:\n>> > Replace all instances of starts_with() by skip_prefix(),\n>> > which can not only be used to check presence of a prefix,\n>> > but also used further on as it returns the string after the prefix,\n>> > if the prefix is present. And also manages to do, what the current\n>> > code does in two steps.\n>>\n>> Better. Thanks.\n>>\n>> > Signed-off-by: Karthik Nayak \n>> > ---\n>> > Hello Eric,\n>> > In this patch, I have:\n>> > 1. Fixed the improper placement of buf_date , initialised to a NULL\n>> > string.\n>> > 2. Fixed whitespace.\n>> > 3. Better naming as per your suggestion.\n>> > 4. Fixed the initilisation before the if statement.\n>> > Thanks for your suggestion.\n>> > ---\n>> > commit.c | 22 +++++++++++-----------\n>> > 1 file changed, 11 insertions(+), 11 deletions(-)\n>> >\n>> > diff --git a/commit.c b/commit.c\n>> > index 6bf4fe0..4144e00 100644\n>> > --- a/commit.c\n>> > +++ b/commit.c\n>> > @@ -553,6 +553,7 @@ static void record_author_date(struct\n>> > author_date_slab *author_date,\n>> > struct ident_split ident;\n>> > char *date_end;\n>> > unsigned long date;\n>> > + const char *buf_date;\n>> >\n>> > if (!commit->buffer) {\n>> > unsigned long size;\n>> > @@ -565,15 +566,15 @@ static void record_author_date(struct\n>> > author_date_slab *author_date,\n>> > for (buf = commit->buffer ? commit->buffer : buffer;\n>> > buf;\n>> > buf = line_end + 1) {\n>> > + buf_date = skip_prefix(buf, \"author \");\n>>\n>> The data after \"author \" is identification information (name, email),\n>> not date. In fact, this information gets passed to the function\n>> split_ident_line(), so a better name for this variable is 'ident_line'\n>> (but not the misspelling 'indent_line' from one of your earlier\n>> attempts).\n>>\n>> > line_end = strchrnul(buf, '\\n');\n>> > - if (!starts_with(buf, \"author \")) {\n>> > + if (!buf_date) {\n>> > if (!line_end[0] || line_end[1] == '\\n')\n>> > return; /* end of header */\n>> > continue;\n>> > }\n>> > - if (split_ident_line(&ident,\n>> > - buf + strlen(\"author \"),\n>> > - line_end - (buf + strlen(\"author\n>> > \"))) ||\n>> > + if (split_ident_line(&ident, buf_date,\n>> > + line_end - buf_date) ||\n>> > !ident.date_begin || !ident.date_end)\n>> > goto fail_exit; /* malformed \"author\" line */\n>> > break;\n>> > @@ -1098,6 +1099,7 @@ int parse_signed_commit(const unsigned char *sha1,\n>> > char *buffer = read_sha1_file(sha1, &type, &size);\n>> > int in_signature, saw_signature = -1;\n>> > char *line, *tail;\n>> > + const char *gpg_sig;\n>> >\n>> > if (!buffer || type != OBJ_COMMIT)\n>> > goto cleanup;\n>> > @@ -1113,9 +1115,9 @@ int parse_signed_commit(const unsigned char *sha1,\n>> > next = next ? next + 1 : tail;\n>> > if (in_signature && line[0] == ' ')\n>> > sig = line + 1;\n>> > - else if (starts_with(line, gpg_sig_header) &&\n>> > - line[gpg_sig_header_len] == ' ')\n>> > - sig = line + gpg_sig_header_len + 1;\n>> > + else if ((gpg_sig = skip_prefix(line, gpg_sig_header))\n>> > + && gpg_sig[0] == ' ')\n>> > + sig = gpg_sig + 1;\n>>\n>> Other than the poor variable name 'buf_date' and the review comments\n>> by Junio and Tanay that this particular change is of questionable\n>> value and perhaps should be dropped, this version of the patch looks\n>> reasonable.\n>>\n>> Thanks.\n>>\n>> > if (sig) {\n>> > strbuf_add(signature, sig, next - sig);\n>> > saw_signature = 1;\n>> > @@ -1193,10 +1195,8 @@ static void parse_gpg_output(struct\n>> > signature_check *sigc)\n>> > for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {\n>> > const char *found, *next;\n>> >\n>> > - if (starts_with(buf, sigcheck_gpg_status[i].check + 1))\n>> > {\n>> > - /* At the very beginning of the buffer */\n>> > - found = buf +\n>> > strlen(sigcheck_gpg_status[i].check + 1);\n>> > - } else {\n>> > + found = skip_prefix(buf, sigcheck_gpg_status[i].check +\n>> > 1);\n>> > + if (!found) {\n>> > found = strstr(buf,\n>> > sigcheck_gpg_status[i].check);\n>> > if (!found)\n>> > continue;\n>> > --\n>> > 1.9.0.138.g2de3478\n>> >\n>\n>\n"},{"id":"236339","messageId":"CAOLa=ZQWREUTSnbm5-XM2yZLR3HXBEkENrHaOPzDFwDx2S3L=g@mail.gmail.com","threadId":"36075","inReplyTo":"CAPig+cS9Eh7bde90SsVWqeWP+fZqDTV3fk6FryoP5SFg-FyfTw@mail.gmail.com","subject":"Re: [PATCH v3] commit.c: Replace starts_with() with skip_prefix()","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2014-03-09T11:10:42Z","receivedAt":"2014-03-09T11:10:42Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Hey Eric,\n\nIts been nice learning from you about how to submit patches to git.\nwas a nice learning curve, now I'm looking into the ideas and will contact the\nappropriate mentor soon with a plan.\n\nThanks\n- Karthik\n\nOn Sun, Mar 9, 2014 at 1:19 PM, Eric Sunshine wrote:\n> On Fri, Mar 7, 2014 at 5:49 AM, karthik nayak wrote:\n>> Hello Eric,\n>> Thanks for your reply, and for that information. should i patch again or\n>> this should do?\n>> And what next? Talk to the mentor?\n>\n> The ultimate authority deciding if a patch is ready is Junio, as it\n> would have to be accepted into his tree. Since he already accepted a\n> similar patch from a different potential GSoC applicant, it may not\n> make sense to refine this one further. What is important is that you\n> are now familiar with the review process on this project, and the\n> mentors (hopefully) have gained insight into your abilities and how\n> you interact with reviewers (which was the goal of these\n> microprojects).\n>\n> Probably best at this point is to consider a proposed project [1] or\n> choose your own, and start the task of applying for a GSoC position\n> (by whatever means that is done).\n>\n> [1]: https://github.com/git/git.github.io/blob/master/SoC-2014-Ideas.md\n>\n>> On Fri, Mar 7, 2014 at 3:04 PM, Eric Sunshine \n>> wrote:\n>>>\n>>> On Thu, Mar 6, 2014 at 12:05 PM, Karthik Nayak \n>>> wrote:\n>>> > Replace all instances of starts_with() by skip_prefix(),\n>>> > which can not only be used to check presence of a prefix,\n>>> > but also used further on as it returns the string after the prefix,\n>>> > if the prefix is present. And also manages to do, what the current\n>>> > code does in two steps.\n>>>\n>>> Better. Thanks.\n>>>\n>>> > Signed-off-by: Karthik Nayak \n>>> > ---\n>>> > Hello Eric,\n>>> > In this patch, I have:\n>>> > 1. Fixed the improper placement of buf_date , initialised to a NULL\n>>> > string.\n>>> > 2. Fixed whitespace.\n>>> > 3. Better naming as per your suggestion.\n>>> > 4. Fixed the initilisation before the if statement.\n>>> > Thanks for your suggestion.\n>>> > ---\n>>> > commit.c | 22 +++++++++++-----------\n>>> > 1 file changed, 11 insertions(+), 11 deletions(-)\n>>> >\n>>> > diff --git a/commit.c b/commit.c\n>>> > index 6bf4fe0..4144e00 100644\n>>> > --- a/commit.c\n>>> > +++ b/commit.c\n>>> > @@ -553,6 +553,7 @@ static void record_author_date(struct\n>>> > author_date_slab *author_date,\n>>> > struct ident_split ident;\n>>> > char *date_end;\n>>> > unsigned long date;\n>>> > + const char *buf_date;\n>>> >\n>>> > if (!commit->buffer) {\n>>> > unsigned long size;\n>>> > @@ -565,15 +566,15 @@ static void record_author_date(struct\n>>> > author_date_slab *author_date,\n>>> > for (buf = commit->buffer ? commit->buffer : buffer;\n>>> > buf;\n>>> > buf = line_end + 1) {\n>>> > + buf_date = skip_prefix(buf, \"author \");\n>>>\n>>> The data after \"author \" is identification information (name, email),\n>>> not date. In fact, this information gets passed to the function\n>>> split_ident_line(), so a better name for this variable is 'ident_line'\n>>> (but not the misspelling 'indent_line' from one of your earlier\n>>> attempts).\n>>>\n>>> > line_end = strchrnul(buf, '\\n');\n>>> > - if (!starts_with(buf, \"author \")) {\n>>> > + if (!buf_date) {\n>>> > if (!line_end[0] || line_end[1] == '\\n')\n>>> > return; /* end of header */\n>>> > continue;\n>>> > }\n>>> > - if (split_ident_line(&ident,\n>>> > - buf + strlen(\"author \"),\n>>> > - line_end - (buf + strlen(\"author\n>>> > \"))) ||\n>>> > + if (split_ident_line(&ident, buf_date,\n>>> > + line_end - buf_date) ||\n>>> > !ident.date_begin || !ident.date_end)\n>>> > goto fail_exit; /* malformed \"author\" line */\n>>> > break;\n>>> > @@ -1098,6 +1099,7 @@ int parse_signed_commit(const unsigned char *sha1,\n>>> > char *buffer = read_sha1_file(sha1, &type, &size);\n>>> > int in_signature, saw_signature = -1;\n>>> > char *line, *tail;\n>>> > + const char *gpg_sig;\n>>> >\n>>> > if (!buffer || type != OBJ_COMMIT)\n>>> > goto cleanup;\n>>> > @@ -1113,9 +1115,9 @@ int parse_signed_commit(const unsigned char *sha1,\n>>> > next = next ? next + 1 : tail;\n>>> > if (in_signature && line[0] == ' ')\n>>> > sig = line + 1;\n>>> > - else if (starts_with(line, gpg_sig_header) &&\n>>> > - line[gpg_sig_header_len] == ' ')\n>>> > - sig = line + gpg_sig_header_len + 1;\n>>> > + else if ((gpg_sig = skip_prefix(line, gpg_sig_header))\n>>> > + && gpg_sig[0] == ' ')\n>>> > + sig = gpg_sig + 1;\n>>>\n>>> Other than the poor variable name 'buf_date' and the review comments\n>>> by Junio and Tanay that this particular change is of questionable\n>>> value and perhaps should be dropped, this version of the patch looks\n>>> reasonable.\n>>>\n>>> Thanks.\n>>>\n>>> > if (sig) {\n>>> > strbuf_add(signature, sig, next - sig);\n>>> > saw_signature = 1;\n>>> > @@ -1193,10 +1195,8 @@ static void parse_gpg_output(struct\n>>> > signature_check *sigc)\n>>> > for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {\n>>> > const char *found, *next;\n>>> >\n>>> > - if (starts_with(buf, sigcheck_gpg_status[i].check + 1))\n>>> > {\n>>> > - /* At the very beginning of the buffer */\n>>> > - found = buf +\n>>> > strlen(sigcheck_gpg_status[i].check + 1);\n>>> > - } else {\n>>> > + found = skip_prefix(buf, sigcheck_gpg_status[i].check +\n>>> > 1);\n>>> > + if (!found) {\n>>> > found = strstr(buf,\n>>> > sigcheck_gpg_status[i].check);\n>>> > if (!found)\n>>> > continue;\n>>> > --\n>>> > 1.9.0.138.g2de3478\n>>> >\n>>\n>>\n"}]}