{"thread":{"id":"36055","subject":"[PATCH v4] commit.c: use skip_prefix() instead of starts_with()","startedAt":"2014-03-04T21:06:30Z","lastAt":"2014-03-05T22:23:13Z","messageCount":5,"participants":["Tanay Abhra","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":null},"messages":[{"id":"236041","messageId":"1393967190-3181-1-git-send-email-tanayabh@gmail.com","threadId":"36055","inReplyTo":null,"subject":"[PATCH v4] commit.c: use skip_prefix() instead of starts_with()","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-03-04T21:06:30Z","receivedAt":"2014-03-04T21:06:30Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"In record_author_date() & parse_gpg_output(), the callers of\nstarts_with() not just want to know if the string starts with the\nprefix, but also can benefit from knowing the string that follows\nthe prefix.\n\nBy using skip_prefix(), we can do both at the same time.\n\nHelped-by: Max Horn \nHelped-by: Junio C Hamano \nHelped-by: Michael Haggerty \nSigned-off-by: Tanay Abhra \nSigned-off-by: Junio C Hamano \n---\nPatch V4 Identation improved, removed useless comment. [1]\n\t\tThanks to Junio C Hamano and Max Horn.\n[1] http://article.gmane.org/gmane.comp.version-control.git/243388\n\nPatch V3 Variable naming improved, removed assignments inside conditionals.\n Thanks to Junio C Hamano and Max Horn.\n\nPatch V2 Corrected email formatting ,reapplied the implementation according to suggestions.\n Thanks to Michael Haggerty.\n\nThis is in respect to GSoC microproject #10.\n\nIn record_author_date(), extra and useless calls to strlen due to using starts_with()\nwere removed by using skip_prefix(). Extra variable \"ident_line\" was used as \"buf\" is used in\nfor loop update check.\n\nOther usages of starts_with() in the same file can be found with,\n\n$ grep -n starts_with commit.c\n\n1116: else if (starts_with(line, gpg_sig_header) &&\n1196: if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {\n\nThe starts_with() in line 1116 was left as it is, as strlen values were pre computed as\nglobal variables, and replacing may hamper the clarity.\nThe starts_with() in line 1196 was replaced as it abstracts way the skip_prefix part by\ndirectly using the function.\nAlso skip_prefix() is inline when compared to starts_with().\n\n commit.c | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 6bf4fe0..01526f7 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);\n static void record_author_date(struct author_date_slab *author_date,\n \t\t\t struct commit *commit)\n {\n-\tconst char *buf, *line_end;\n+\tconst char *buf, *line_end, *ident_line;\n \tchar *buffer = NULL;\n \tstruct ident_split ident;\n \tchar *date_end;\n@@ -566,14 +566,14 @@ static void record_author_date(struct author_date_slab *author_date,\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\tident_line = skip_prefix(buf, \"author \");\n+\t\tif (!ident_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\t\t\t \t\t ident_line, line_end - ident_line) ||\n \t\t !ident.date_begin || !ident.date_end)\n \t\t\tgoto fail_exit; /* malformed \"author\" line */\n \t\tbreak;\n@@ -1193,10 +1193,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\n"},{"id":"236043","messageId":"xmqqzjl5iro0.fsf@gitster.dls.corp.google.com","threadId":"36055","inReplyTo":"1393967190-3181-1-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH v4] commit.c: use skip_prefix() instead of starts_with()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-04T21:23:59Z","receivedAt":"2014-03-04T21:23:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tanay Abhra writes:\n\n> In record_author_date() & parse_gpg_output(), the callers of\n> starts_with() not just want to know if the string starts with the\n> prefix, but also can benefit from knowing the string that follows\n> the prefix.\n>\n> By using skip_prefix(), we can do both at the same time.\n>\n> Helped-by: Max Horn \n> Helped-by: Junio C Hamano \n> Helped-by: Michael Haggerty \n> Signed-off-by: Tanay Abhra \n> Signed-off-by: Junio C Hamano \n\nDo not add the last when sending; I never signed-off this particular\nversion of this patch.\n\n> diff --git a/commit.c b/commit.c\n> index 6bf4fe0..01526f7 100644\n> --- a/commit.c\n> +++ b/commit.c\n> @@ -548,7 +548,7 @@ define_commit_slab(author_date_slab, unsigned long);\n> static void record_author_date(struct author_date_slab *author_date,\n> \t\t\t struct commit *commit)\n> {\n> -\tconst char *buf, *line_end;\n> +\tconst char *buf, *line_end, *ident_line;\n> \tchar *buffer = NULL;\n> \tstruct ident_split ident;\n> \tchar *date_end;\n> @@ -566,14 +566,14 @@ static void record_author_date(struct author_date_slab *author_date,\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\tident_line = skip_prefix(buf, \"author \");\n> +\t\tif (!ident_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\t\t\t \t\t ident_line, line_end - ident_line) ||\n\nFunny indentation with some SP followed by HT followed by SP.\n\n> \t\t !ident.date_begin || !ident.date_end)\n> \t\t\tgoto fail_exit; /* malformed \"author\" line */\n> \t\tbreak;\n> @@ -1193,10 +1193,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"},{"id":"236046","messageId":"xmqqvbvtiph8.fsf@gitster.dls.corp.google.com","threadId":"36055","inReplyTo":"xmqqzjl5iro0.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4] commit.c: use skip_prefix() instead of starts_with()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-04T22:11:15Z","receivedAt":"2014-03-04T22:11:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> +\t\tfound = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);\n>> +\t\tif(!found) {\n\nMissing SP between the control keyword and parenthesized expression\nthe keyword uses.\n\nI've fixed this (and the broken indentation) locally and queued the\nresult to 'pu', so no need to resend to correct this one.\n\nThanks.\n"},{"id":"236051","messageId":"loom.20140304T233205-398@post.gmane.org","threadId":"36055","inReplyTo":"xmqqvbvtiph8.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v4] commit.c: use skip_prefix() instead of starts_with()","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-03-04T22:32:30Z","receivedAt":"2014-03-04T22:32:30Z","isPatch":true,"sender":{"key":"tanayabh@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2097229?v=4"},"body":"Junio C Hamano pobox.com> writes:\n\n> \n> Junio C Hamano pobox.com> writes:\n> \n> >> +\t\tfound = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);\n> >> +\t\tif(!found) {\n> \n> Missing SP between the control keyword and parenthesized expression\n> the keyword uses.\n> \n> I've fixed this (and the broken indentation) locally and queued the\n> result to 'pu', so no need to resend to correct this one.\n> \n> Thanks.\n> \n> \n\nSorry about the indentation, it was due to not setting the tab to eight\nspaces. I found your mail late, so I had already\nsent a revised patch [1]. Please ignore that mail.\n\nAlso , what should be my next step ,should I present a rough draft of a\nproposal , or tackle other bugs on the mailing list?\n\nThanks for the suggestions and advice,\n\nRegards,\nTanay Abhra.\n\n[1] http://article.gmane.org/gmane.comp.version-control.git/243395\n"},{"id":"236135","messageId":"xmqqd2i02sku.fsf@gitster.dls.corp.google.com","threadId":"36055","inReplyTo":"CAEc54XB3hdsJcO4QFuqFHi+mZtZDrV2wFdMQ1uHiPrxzfo99jw@mail.gmail.com","subject":"Re: [PATCH v4] commit.c: use skip_prefix() instead of starts_with()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-05T22:23:13Z","receivedAt":"2014-03-05T22:23:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"tanay abhra writes:\n\n> On Wed, Mar 5, 2014 at 3:41 AM, Junio C Hamano wrote:\n>\n>> Junio C Hamano writes:\n>>\n>> >> + found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);\n>> >> + if(!found) {\n>>\n>> Missing SP between the control keyword and parenthesized expression\n>> the keyword uses.\n>>\n>> I've fixed this (and the broken indentation) locally and queued the\n>> result to 'pu', so no need to resend to correct this one.\n>>\n>> Thanks.\n>>\n>>\n> Sorry about the indentation, it was due to not setting the tab to eight\n> spaces. I found your mail late, so I had already\n> sent a revised patch [1]. Please ignore that mail.\n>\n> Also , what should be my next step ,should I present a rough draft of a\n> proposal , or tackle other bugs on the mailing list?\n\nAs far as I, as the maintainer of the project, am concerned [*1*],\nwe are done and I expect/require nothing more from you on this\nchange.\n\nThe MicroProject page says:\n\n ... If you've already done a microproject and are itching to do\n more, then get involved in other ways, like finding and fixing\n other problems in the code, or improving the documentation or\n code comments, or helping to review other people's patches on\n the mailing list, or answering questions on the mailing list or\n in IRC, or writing new tests, etc., etc. In short, start doing\n things that other Git developers do!\n\nFYI, [GSoC 2014 timeline] (Google for it) tells us that would-be\nstudents are supposed to be discussing project ideas with their\nmentoring organizations now, in preparation for the actual student\napplication that begins on Mar 10 and ends on Mar 21.\n\n\n[Footnote]\n\n*1* I should mention that I am not involved in GSoC administration\nand student selection for the Git project.\n"}]}