{"thread":{"id":"36057","subject":"[PATCH v5] commit.c: use skip_prefix() instead of starts_with()","startedAt":"2014-03-04T22:07:11Z","lastAt":"2014-03-21T15:53:36Z","messageCount":3,"participants":["Tanay Abhra","Michael Haggerty"],"isPatch":true,"patchVersion":5,"patchTotal":null},"messages":[{"id":"236045","messageId":"1393970831-3558-1-git-send-email-tanayabh@gmail.com","threadId":"36057","inReplyTo":null,"subject":"[PATCH v5] commit.c: use skip_prefix() instead of starts_with()","fromName":"Tanay Abhra","fromEmail":"tanayabh@gmail.com","sentAt":"2014-03-04T22:07:11Z","receivedAt":"2014-03-04T22:07:11Z","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 \n---\nPatch V5 Minor revision of indentation\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---\n commit.c | 14 ++++++--------\n 1 file changed, 6 insertions(+), 8 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 6bf4fe0..d37675c 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 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":"237315","messageId":"532C5F47.3020801@alum.mit.edu","threadId":"36057","inReplyTo":"1393970831-3558-1-git-send-email-tanayabh@gmail.com","subject":"Re: [PATCH v5] commit.c: use skip_prefix() instead of starts_with()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-21T15:48:23Z","receivedAt":"2014-03-21T15:48:23Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 03/04/2014 11:07 PM, Tanay Abhra wrote:\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> ---\n> Patch V5 Minor revision of indentation\n> Patch 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> \n> Patch V3 Variable naming improved, removed assignments inside conditionals.\n> Thanks to Junio C Hamano and Max Horn.\n> \n> Patch V2 Corrected email formatting ,reapplied the implementation according to suggestions.\n> Thanks to Michael Haggerty.\n> \n> This is in respect to GSoC microproject #10.\n> \n> In record_author_date(), extra and useless calls to strlen due to using starts_with()\n> were removed by using skip_prefix(). Extra variable \"ident_line\" was used as \"buf\" is used in\n> for loop update check.\n> \n> Other usages of starts_with() in the same file can be found with,\n> \n> $ grep -n starts_with commit.c\n> \n> 1116: else if (starts_with(line, gpg_sig_header) &&\n> 1196: if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {\n> \n> The starts_with() in line 1116 was left as it is, as strlen values were pre computed as\n> global variables, and replacing may hamper the clarity.\n> The starts_with() in line 1196 was replaced as it abstracts way the skip_prefix part by\n> directly using the function.\n> Also skip_prefix() is inline when compared to starts_with().\n> \n> ---\n> commit.c | 14 ++++++--------\n> 1 file changed, 6 insertions(+), 8 deletions(-)\n> \n> diff --git a/commit.c b/commit.c\n> index 6bf4fe0..d37675c 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 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\nNit: There should be a space between \"if\" and the opening parenthesis.\n\n> \t\t\tfound = strstr(buf, sigcheck_gpg_status[i].check);\n> \t\t\tif (!found)\n> \t\t\t\tcontinue;\n> \n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"237316","messageId":"532C6080.9010503@alum.mit.edu","threadId":"36057","inReplyTo":"532C5F47.3020801@alum.mit.edu","subject":"Re: [PATCH v5] commit.c: use skip_prefix() instead of starts_with()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-03-21T15:53:36Z","receivedAt":"2014-03-21T15:53:36Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 03/21/2014 04:48 PM, Michael Haggerty wrote:\n> On 03/04/2014 11:07 PM, Tanay Abhra wrote:\n>> [...]\n>> +\t\tfound = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);\n>> +\t\tif(!found) {\n> \n> Nit: There should be a space between \"if\" and the opening parenthesis.\n\nOops, I see I am too late. Junio must have fixed this when queuing the\npatch.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}