{"thread":{"id":"42330","subject":"[PATCH] commit.c: use strchrnul() to scan for one line","startedAt":"2016-05-15T22:23:15Z","lastAt":"2016-05-16T15:57:59Z","messageCount":3,"participants":["Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"286611","messageId":"xmqq37pj55bg.fsf@gitster.mtv.corp.google.com","threadId":"42330","inReplyTo":null,"subject":"[PATCH] commit.c: use strchrnul() to scan for one line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-15T22:23:15Z","receivedAt":"2016-05-15T22:23:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n commit.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex 3f4f371..1f9ee8a 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -415,8 +415,7 @@ int find_commit_subject(const char *commit_buffer, const char **subject)\n \t\tp++;\n \tif (*p) {\n \t\tp += 2;\n-\t\tfor (eol = p; *eol && *eol != '\\n'; eol++)\n-\t\t\t; /* do nothing */\n+\t\teol = strchrnul(p, '\\n');\n \t} else\n \t\teol = p;\n \n-- \n2.8.2-748-gfb85f76\n"},{"id":"286645","messageId":"alpine.DEB.2.20.1605160810020.3303@virtualbox","threadId":"42330","inReplyTo":"xmqq37pj55bg.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] commit.c: use strchrnul() to scan for one line","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-05-16T06:12:33Z","receivedAt":"2016-05-16T06:12:33Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Sun, 15 May 2016, Junio C Hamano wrote:\n\n> diff --git a/commit.c b/commit.c\n> index 3f4f371..1f9ee8a 100644\n> --- a/commit.c\n> +++ b/commit.c\n> @@ -415,8 +415,7 @@ int find_commit_subject(const char *commit_buffer, const char **subject)\n>  \t\tp++;\n>  \tif (*p) {\n>  \t\tp += 2;\n> -\t\tfor (eol = p; *eol && *eol != '\\n'; eol++)\n> -\t\t\t; /* do nothing */\n> +\t\teol = strchrnul(p, '\\n');\n>  \t} else\n>  \t\teol = p;\n\nACK. This was my fault, when I introduced the code in 9509af68 (Make\ngit-revert & git-cherry-pick a builtin, 2007-03-01). To be fair,\nstrchrnul() was introduced only later, in 659c69c (Add strchrnul(),\n2007-11-09).\n\nCiao,\nDscho\n"},{"id":"286668","messageId":"xmqq1t523shk.fsf@gitster.mtv.corp.google.com","threadId":"42330","inReplyTo":"alpine.DEB.2.20.1605160810020.3303@virtualbox","subject":"Re: [PATCH] commit.c: use strchrnul() to scan for one line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-16T15:57:59Z","receivedAt":"2016-05-16T15:57:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> On Sun, 15 May 2016, Junio C Hamano wrote:\n>\n>> diff --git a/commit.c b/commit.c\n>> index 3f4f371..1f9ee8a 100644\n>> --- a/commit.c\n>> +++ b/commit.c\n>> @@ -415,8 +415,7 @@ int find_commit_subject(const char *commit_buffer, const char **subject)\n>>  \t\tp++;\n>>  \tif (*p) {\n>>  \t\tp += 2;\n>> -\t\tfor (eol = p; *eol && *eol != '\\n'; eol++)\n>> -\t\t\t; /* do nothing */\n>> +\t\teol = strchrnul(p, '\\n');\n>>  \t} else\n>>  \t\teol = p;\n>\n> ACK. This was my fault, when I introduced the code in 9509af68 (Make\n> git-revert & git-cherry-pick a builtin, 2007-03-01). To be fair,\n> strchrnul() was introduced only later, in 659c69c (Add strchrnul(),\n> 2007-11-09).\n\nOh, there is no fault.\n\nI was reading through attr.c to see how bad a work to revamp\nattribute lookup would look like, found a hand-rolled strchrnul()\nthat predates the widespread use of the function, and looked for\nsimilar patterns while I was updating it.  As there were many false\npositives (i.e. \"Yes, this loop is looking for the end of line, but\nit does something else to the characters on the line while doing so,\nso it cannot become strchrnul()\") that I needed eyeballing in order\nto reject and avoid incorrect conversion, it is very much appreciated\nthat you double-checked this one that I spotted.\n\nThanks.\n"}]}