{"thread":{"id":"36051","subject":"[PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()","startedAt":"2014-03-04T15:54:05Z","lastAt":"2014-03-05T07:48:04Z","messageCount":7,"participants":["Karthik Nayak","Tanay Abhra","karthik nayak","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"236001","messageId":"1393948445-24689-1-git-send-email-karthik.188@gmail.com","threadId":"36051","inReplyTo":null,"subject":"[PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2014-03-04T15:54:05Z","receivedAt":"2014-03-04T15:54:05Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In record_author_date() :\nReplace \"buf + strlen(author )\" by skip_prefix(), which is\nsaved in a new \"const char\" variable \"indent_line\".\n\nIn parse_signed_commit() :\nReplace \"line + gpg_sig_header_len\" by skip_prefix(), which\nis saved in a new \"const char\" variable \"indent_line\".\n\nIn parse_gpg_output() :\nReplace \"buf + strlen(sigcheck_gpg_status[i].check + 1)\" by\nskip_prefix(), which is saved in already available\nvariable \"found\".\n\nSigned-off-by: Karthik Nayak \n---\n commit.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 6bf4fe0..71a03e3 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 *indent_line;\n \n \tif (!commit->buffer) {\n \t\tunsigned long size;\n@@ -562,18 +563,19 @@ static void record_author_date(struct author_date_slab *author_date,\n \t\t\treturn;\n \t}\n \n+\tindent_line = skip_prefix(buf, \"author \");\n+\n \tfor (buf = commit->buffer ? commit->buffer : buffer;\n \t buf;\n \t buf = line_end + 1) {\n \t\tline_end = strchrnul(buf, '\\n');\n-\t\tif (!starts_with(buf, \"author \")) {\n+\t\tif (!indent_line) {\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, indent_line,\n+\t\t\t\t\t\t\t line_end - indent_line) ||\n \t\t !ident.date_begin || !ident.date_end)\n \t\t\tgoto fail_exit; /* malformed \"author\" line */\n \t\tbreak;\n@@ -1098,6 +1100,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 *indent_line;\n \n \tif (!buffer || type != OBJ_COMMIT)\n \t\tgoto cleanup;\n@@ -1111,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1,\n \t\tchar *next = memchr(line, '\\n', tail - line);\n \n \t\tnext = next ? next + 1 : tail;\n+\t\tindent_line = skip_prefix(line, gpg_sig_header);\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 (indent_line && indent_line[1] == ' ')\n+\t\t\tsig = indent_line + 2;\n \t\tif (sig) {\n \t\t\tstrbuf_add(signature, sig, next - sig);\n \t\t\tsaw_signature = 1;\n@@ -1193,10 +1196,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\nHey Eric,\nAs per your suggestion in the previous mail :\nhttp://article.gmane.org/gmane.comp.version-control.git/243316\nI have made necessary changes:\n1. Better Naming of variables as per suggestion\n2. Proper replacement of skip_prefix() over starts_with() .\n"},{"id":"236010","messageId":"loom.20140304T182823-503@post.gmane.org","threadId":"36051","inReplyTo":"1393948445-24689-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-03-04T17:31:12Z","receivedAt":"2014-03-04T17:31:12Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"\nKarthik Nayak gmail.com> writes:\n\n> \n> In record_author_date() :\n> Replace \"buf + strlen(author )\" by skip_prefix(), which is\n> saved in a new \"const char\" variable \"indent_line\".\n> \n> In parse_signed_commit() :\n> Replace \"line + gpg_sig_header_len\" by skip_prefix(), which\n> is saved in a new \"const char\" variable \"indent_line\".\n> \n> In parse_gpg_output() :\n> Replace \"buf + strlen(sigcheck_gpg_status[i].check + 1)\" by\n> skip_prefix(), which is saved in already available\n> variable \"found\".\n> \n> Signed-off-by: Karthik Nayak gmail.com>\n> ---\n> commit.c | 23 ++++++++++++-----------\n> 1 file changed, 12 insertions(+), 11 deletions(-)\n> \n> diff --git a/commit.c b/commit.c\n> index 6bf4fe0..71a03e3 100644\n> --- a/commit.c\n> +++ b/commit.c\n\n> -1098,6 +1100,7 int parse_signed_commit(const\nunsigned 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 *indent_line;\n> \n> \tif (!buffer || type != OBJ_COMMIT)\n> \t\tgoto cleanup;\n> -1111,11 +1114,11 int parse_signed_commit(const\nunsigned char *sha1,\n> \t\tchar *next = memchr(line, '\\n', tail - line);\n> \n> \t\tnext = next ? next + 1 : tail;\n> +\t\tindent_line = skip_prefix(line, gpg_sig_header);\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 (indent_line && indent_line[1] == ' ')\n> +\t\t\tsig = indent_line + 2;\n> \t\tif (sig) {\n> \t\t\tstrbuf_add(signature, sig, next - sig);\n> \t\t\tsaw_signature = 1;\n\n\nHi,\n\nI was attempting the same microproject so I thought I would share some\npoints that were told to me earlier .The mail subject should have a\nincreasing order of submission numbers for each revision(for example here it\nshould be [PATCH v2]).\n\nAlso write the superfluous stuff which you have written in the bottom \nafter ---(the three dashes after the signed off statement) .\n\nIn the parse_signed_commit() function, gpg_sig_header_len and gpg_sig_header\nvariables are precomputed outside of the function, and also Junio said to\nprefer starts_with(), whenever skip_prefix() advantages are not so important\nin the context.So the replace may not be so advantageous ,I may be wrong in\nthis case.\n\n \n\nCheers,\nTanay Abhra.\n"},{"id":"236013","messageId":"CAOLa=ZSPeKUyBZ2gJHByhg8QxTdwScfKGA6gdxREvyjZwL0hsw@mail.gmail.com","threadId":"36051","inReplyTo":"loom.20140304T182823-503@post.gmane.org","subject":"Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()","fromName":"karthik nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2014-03-04T17:58:07Z","receivedAt":"2014-03-04T17:58:07Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Hey Tanay,\n\n1. Yes just getting used to git send-email now, should follow that from now\n2. I thought it shouldn't be a part of the commit, so i put it after\nthe last ---\n3. I did have a thought on your lines also , but concluded to it being\nmore advantageous, you might be right though\n\nNice to hear from you.\nThanks\n-Karthik Nayak\n\nOn Tue, Mar 4, 2014 at 11:01 PM, Tanay Abhra wrote:\n>\n> Karthik Nayak gmail.com> writes:\n>\n>>\n>> In record_author_date() :\n>> Replace \"buf + strlen(author )\" by skip_prefix(), which is\n>> saved in a new \"const char\" variable \"indent_line\".\n>>\n>> In parse_signed_commit() :\n>> Replace \"line + gpg_sig_header_len\" by skip_prefix(), which\n>> is saved in a new \"const char\" variable \"indent_line\".\n>>\n>> In parse_gpg_output() :\n>> Replace \"buf + strlen(sigcheck_gpg_status[i].check + 1)\" by\n>> skip_prefix(), which is saved in already available\n>> variable \"found\".\n>>\n>> Signed-off-by: Karthik Nayak gmail.com>\n>> ---\n>> commit.c | 23 ++++++++++++-----------\n>> 1 file changed, 12 insertions(+), 11 deletions(-)\n>>\n>> diff --git a/commit.c b/commit.c\n>> index 6bf4fe0..71a03e3 100644\n>> --- a/commit.c\n>> +++ b/commit.c\n>\n>> -1098,6 +1100,7 int parse_signed_commit(const\n> 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 *indent_line;\n>>\n>> if (!buffer || type != OBJ_COMMIT)\n>> goto cleanup;\n>> -1111,11 +1114,11 int parse_signed_commit(const\n> unsigned char *sha1,\n>> char *next = memchr(line, '\\n', tail - line);\n>>\n>> next = next ? next + 1 : tail;\n>> + indent_line = skip_prefix(line, gpg_sig_header);\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 (indent_line && indent_line[1] == ' ')\n>> + sig = indent_line + 2;\n>> if (sig) {\n>> strbuf_add(signature, sig, next - sig);\n>> saw_signature = 1;\n>\n>\n> Hi,\n>\n> I was attempting the same microproject so I thought I would share some\n> points that were told to me earlier .The mail subject should have a\n> increasing order of submission numbers for each revision(for example here it\n> should be [PATCH v2]).\n>\n> Also write the superfluous stuff which you have written in the bottom\n> after ---(the three dashes after the signed off statement) .\n>\n> In the parse_signed_commit() function, gpg_sig_header_len and gpg_sig_header\n> variables are precomputed outside of the function, and also Junio said to\n> prefer starts_with(), whenever skip_prefix() advantages are not so important\n> in the context.So the replace may not be so advantageous ,I may be wrong in\n> this case.\n>\n>\n>\n> Cheers,\n> Tanay Abhra.\n>\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"236049","messageId":"CAPig+cR=9oonOdyOm3y2NpdLxh-sq5qQtSxmmN2dH7+UzRy-gA@mail.gmail.com","threadId":"36051","inReplyTo":"1393948445-24689-1-git-send-email-karthik.188@gmail.com","subject":"Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-04T22:27:11Z","receivedAt":"2014-03-04T22:27:11Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Thanks for the resend. Etiquette on this list is to cc: people who\ncommented on previous versions of the submission. As Tanay already\nmentioned, use [PATCH vN] in the subject where N is the version number\nof this attempt. The -v option of \"git format-email\" can help.\n\nMore below.\n\nOn Tue, Mar 4, 2014 at 10:54 AM, Karthik Nayak wrote:\n> In record_author_date() :\n> Replace \"buf + strlen(author )\" by skip_prefix(), which is\n> saved in a new \"const char\" variable \"indent_line\".\n>\n> In parse_signed_commit() :\n> Replace \"line + gpg_sig_header_len\" by skip_prefix(), which\n> is saved in a new \"const char\" variable \"indent_line\".\n>\n> In parse_gpg_output() :\n> Replace \"buf + strlen(sigcheck_gpg_status[i].check + 1)\" by\n> skip_prefix(), which is saved in already available\n> variable \"found\".\n\nIt's not necessary to explain in prose what the patch itself already\nstates more concisely and precisely. All of this text should be\ndropped from the commit message. Instead, explain the purpose of the\npatch, the problem it solves, etc. Please read the \"(2) Describe your\nchanges well\" section of Documentation/SubmittingPatches to get an\nidea of what sort of information to include in the commit message.\n\nA better commit message should say something about how the affected\nfunctions want to know not only if the string has a prefix, but also\nthe text following the prefix, and that skip_prefix() can provide both\npieces of information.\n\n> Signed-off-by: Karthik Nayak \n> ---\n> commit.c | 23 ++++++++++++-----------\n> 1 file changed, 12 insertions(+), 11 deletions(-)\n>\n> diff --git a/commit.c b/commit.c\n> index 6bf4fe0..71a03e3 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 *indent_line;\n\nStrange variable name. The code in question splits apart a person's\nidentification string (name, email, etc.). It has nothing to do with\nindentation.\n\n> if (!commit->buffer) {\n> unsigned long size;\n> @@ -562,18 +563,19 @@ static void record_author_date(struct author_date_slab *author_date,\n> return;\n> }\n>\n> + indent_line = skip_prefix(buf, \"author \");\n> +\n> for (buf = commit->buffer ? commit->buffer : buffer;\n> buf;\n> buf = line_end + 1) {\n> line_end = strchrnul(buf, '\\n');\n> - if (!starts_with(buf, \"author \")) {\n> + if (!indent_line) {\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, indent_line,\n> + line_end - indent_line) ||\n\nIndent the second line (using tabs plus possible spaces) so that it\nlines up with the &ident in the line above. Be sure to set your editor\nso tabs have width 8.\n\n> !ident.date_begin || !ident.date_end)\n> goto fail_exit; /* malformed \"author\" line */\n> break;\n> @@ -1098,6 +1100,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 *indent_line;\n>\n> if (!buffer || type != OBJ_COMMIT)\n> goto cleanup;\n> @@ -1111,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1,\n> char *next = memchr(line, '\\n', tail - line);\n>\n> next = next ? next + 1 : tail;\n> + indent_line = skip_prefix(line, gpg_sig_header);\n\nEven stranger variable name for a GPG signature (which has nothing at\nall to do with indentation).\n\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 (indent_line && indent_line[1] == ' ')\n> + sig = indent_line + 2;\n\nWhy is this adding 2 rather than 1?\n\n> if (sig) {\n> strbuf_add(signature, sig, next - sig);\n> saw_signature = 1;\n> @@ -1193,10 +1196,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\nAdd a space after the plus sign.\n\n> + if(!found) {\n> found = strstr(buf, sigcheck_gpg_status[i].check);\n\n'found' is being computed again here, even though you already computed\nit just above via skip_prefix(). This seems pretty wasteful.\n\n> if (!found)\n> continue;\n> --\n> 1.9.0.138.g2de3478\n>\n> Hey Eric,\n> As per your suggestion in the previous mail :\n> http://article.gmane.org/gmane.comp.version-control.git/243316\n> I have made necessary changes:\n> 1. Better Naming of variables as per suggestion\n> 2. Proper replacement of skip_prefix() over starts_with() .\n\nThanks for explaining the changes since the previous version and\nproviding a link. That's good etiquette.\n\nAs Tanay already mentioned in his review, place this commentary just\nbelow the \"---\" line following your sign off. (You placed it below the\n\"--\" line.)\n"},{"id":"236050","messageId":"CAPig+cTR8PfRSTAMxCbnHbHGT4fn5xEO=iaOTQ0MQ0KSsy-gDQ@mail.gmail.com","threadId":"36051","inReplyTo":"CAOLa=ZSPeKUyBZ2gJHByhg8QxTdwScfKGA6gdxREvyjZwL0hsw@mail.gmail.com","subject":"Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-04T22:30:09Z","receivedAt":"2014-03-04T22:30:09Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 4, 2014 at 12:58 PM, karthik nayak wrote:\n> Hey Tanay,\n>\n> 1. Yes just getting used to git send-email now, should follow that from now\n> 2. I thought it shouldn't be a part of the commit, so i put it after\n> the last ---\n> 3. I did have a thought on your lines also , but concluded to it being\n> more advantageous, you might be right though\n\nMinor etiquette note: On this list, respond inline rather than top-posting.\n\nTo understand why, look at how much scrolling up and down a person has\nto do to relate your points 1, 2, and 3 to review statements made by\nTanay at the very bottom of the email.\n\n> Nice to hear from you.\n> Thanks\n> -Karthik Nayak\n>\n> On Tue, Mar 4, 2014 at 11:01 PM, Tanay Abhra wrote:\n>>\n>> Karthik Nayak gmail.com> writes:\n>>\n>>>\n>>> In record_author_date() :\n>>> Replace \"buf + strlen(author )\" by skip_prefix(), which is\n>>> saved in a new \"const char\" variable \"indent_line\".\n>>>\n>>> In parse_signed_commit() :\n>>> Replace \"line + gpg_sig_header_len\" by skip_prefix(), which\n>>> is saved in a new \"const char\" variable \"indent_line\".\n>>>\n>>> In parse_gpg_output() :\n>>> Replace \"buf + strlen(sigcheck_gpg_status[i].check + 1)\" by\n>>> skip_prefix(), which is saved in already available\n>>> variable \"found\".\n>>>\n>>> Signed-off-by: Karthik Nayak gmail.com>\n>>> ---\n>>> commit.c | 23 ++++++++++++-----------\n>>> 1 file changed, 12 insertions(+), 11 deletions(-)\n>>>\n>>> diff --git a/commit.c b/commit.c\n>>> index 6bf4fe0..71a03e3 100644\n>>> --- a/commit.c\n>>> +++ b/commit.c\n>>\n>>> -1098,6 +1100,7 int parse_signed_commit(const\n>> 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 *indent_line;\n>>>\n>>> if (!buffer || type != OBJ_COMMIT)\n>>> goto cleanup;\n>>> -1111,11 +1114,11 int parse_signed_commit(const\n>> unsigned char *sha1,\n>>> char *next = memchr(line, '\\n', tail - line);\n>>>\n>>> next = next ? next + 1 : tail;\n>>> + indent_line = skip_prefix(line, gpg_sig_header);\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 (indent_line && indent_line[1] == ' ')\n>>> + sig = indent_line + 2;\n>>> if (sig) {\n>>> strbuf_add(signature, sig, next - sig);\n>>> saw_signature = 1;\n>>\n>>\n>> Hi,\n>>\n>> I was attempting the same microproject so I thought I would share some\n>> points that were told to me earlier .The mail subject should have a\n>> increasing order of submission numbers for each revision(for example here it\n>> should be [PATCH v2]).\n>>\n>> Also write the superfluous stuff which you have written in the bottom\n>> after ---(the three dashes after the signed off statement) .\n>>\n>> In the parse_signed_commit() function, gpg_sig_header_len and gpg_sig_header\n>> variables are precomputed outside of the function, and also Junio said to\n>> prefer starts_with(), whenever skip_prefix() advantages are not so important\n>> in the context.So the replace may not be so advantageous ,I may be wrong in\n>> this case.\n>>\n>>\n>>\n>> Cheers,\n>> Tanay Abhra.\n>>\n>>\n>> --\n>> To unsubscribe from this list: send the line \"unsubscribe git\" in\n>> the body of a message to majordomo@vger.kernel.org\n>> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"236052","messageId":"CAPig+cRna0FX66535HX2Yz6t4eZ_zcJDa5HWBJ2nphKA+Aqi8g@mail.gmail.com","threadId":"36051","inReplyTo":"CAPig+cR=9oonOdyOm3y2NpdLxh-sq5qQtSxmmN2dH7+UzRy-gA@mail.gmail.com","subject":"Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-04T22:43:15Z","receivedAt":"2014-03-04T22:43:15Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 4, 2014 at 5:27 PM, Eric Sunshine wrote:\n> On Tue, Mar 4, 2014 at 10:54 AM, Karthik Nayak wrote:\n>> diff --git a/commit.c b/commit.c\n>> index 6bf4fe0..71a03e3 100644\n>> --- a/commit.c\n>> +++ b/commit.c\n>> @@ -1193,10 +1196,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>\n> Add a space after the plus sign.\n>\n>> + if(!found) {\n>> found = strstr(buf, sigcheck_gpg_status[i].check);\n>\n> 'found' is being computed again here, even though you already computed\n> it just above via skip_prefix(). This seems pretty wasteful.\n\nIgnore this objection. I must have misread the code as I was composing\nthe email.\n"},{"id":"236072","messageId":"CAPig+cSQtQ_uvjdA2s3+K8YQLDuMXqqc-jG2AuSM7K52q3_pPQ@mail.gmail.com","threadId":"36051","inReplyTo":"CAPig+cR=9oonOdyOm3y2NpdLxh-sq5qQtSxmmN2dH7+UzRy-gA@mail.gmail.com","subject":"Re: [PATCH] [PATCH] commit.c: Replace starts_with() with skip_prefix()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-05T07:48:04Z","receivedAt":"2014-03-05T07:48:04Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 4, 2014 at 5:27 PM, Eric Sunshine wrote:\n> On Tue, Mar 4, 2014 at 10:54 AM, Karthik Nayak wrote:\n>> diff --git a/commit.c b/commit.c\n>> index 6bf4fe0..71a03e3 100644\n>> --- a/commit.c\n>> +++ b/commit.c\n>> @@ -1111,11 +1114,11 @@ int parse_signed_commit(const unsigned char *sha1,\n>> char *next = memchr(line, '\\n', tail - line);\n>>\n>> next = next ? next + 1 : tail;\n>> + indent_line = skip_prefix(line, gpg_sig_header);\n>\n> Even stranger variable name for a GPG signature (which has nothing at\n> all to do with indentation).\n>\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 (indent_line && indent_line[1] == ' ')\n\nAlso, shouldn't this be checking *indent_line (or indent_line[0])\nrather than indent_line[1]?\n\n>> + sig = indent_line + 2;\n>\n> Why is this adding 2 rather than 1?\n>\n>> if (sig) {\n>> strbuf_add(signature, sig, next - sig);\n>> saw_signature = 1;\n"}]}