{"thread":{"id":"11197","subject":"[PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer.","startedAt":"2007-12-08T15:48:05Z","lastAt":"2007-12-09T19:24:37Z","messageCount":3,"participants":["Jim Meyering","Junio C Hamano","Brian Gernhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"62418","messageId":"87hcitgpca.fsf@rho.meyering.net","threadId":"11197","inReplyTo":null,"subject":"[PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer.","fromName":"Jim Meyering","fromEmail":"jim@meyering.net","sentAt":"2007-12-08T15:48:05Z","receivedAt":"2007-12-08T15:48:05Z","isPatch":true,"sender":{"key":"jim@meyering.net","avatar":"https://avatars.githubusercontent.com/u/710630?v=4"},"body":"\n* config.c (store_write_pair): Don't read value[-1].\n\nSigned-off-by: Jim Meyering <meyering@redhat.com>\n---\n config.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex ed96213..6031b38 100644\n--- a/config.c\n+++ b/config.c\n@@ -652,7 +652,7 @@ static int store_write_pair(int fd, const char* key, const char* value)\n \tfor (i = 0; value[i]; i++)\n \t\tif (value[i] == ';' || value[i] == '#')\n \t\t\tquote = 1;\n-\tif (value[i-1] == ' ')\n+\tif (i && value[i-1] == ' ')\n \t\tquote = 1;\n\n \tif (write_in_full(fd, \"\\t\", 1) != 1 ||\n--\n1.5.3.7.1116.gae2a9\n"},{"id":"62501","messageId":"7vsl2bpwe1.fsf@gitster.siamese.dyndns.org","threadId":"11197","inReplyTo":"87hcitgpca.fsf@rho.meyering.net","subject":"Re: [PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-09T18:15:34Z","receivedAt":"2007-12-09T18:15:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jim Meyering <jim@meyering.net> writes:\n\n> * config.c (store_write_pair): Don't read value[-1].\n>\n> Signed-off-by: Jim Meyering <meyering@redhat.com>\n> ---\n>  config.c |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/config.c b/config.c\n> index ed96213..6031b38 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -652,7 +652,7 @@ static int store_write_pair(int fd, const char* key, const char* value)\n>  \tfor (i = 0; value[i]; i++)\n>  \t\tif (value[i] == ';' || value[i] == '#')\n>  \t\t\tquote = 1;\n> -\tif (value[i-1] == ' ')\n> +\tif (i && value[i-1] == ' ')\n>  \t\tquote = 1;\n>\n>  \tif (write_in_full(fd, \"\\t\", 1) != 1 ||\n\nThanks.\n\nThe fix may be correct but the comment above the part the patch fixes is\nvery misleading and I wasted a few minutes checking to see what it is\ndoing is correct.\n\nThe \"quote\" variable does not really control quoting; the quoting with\nbackslash is always done for the value, and this check is about adding a\ndq pair around it, so that the parser does not lose leading or trailing\nSP or string after start-of-comment marker characters.\n"},{"id":"62513","messageId":"33716C48-068F-42C5-BD84-608EE9017636@silverinsanity.com","threadId":"11197","inReplyTo":"7vsl2bpwe1.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer.","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2007-12-09T19:24:37Z","receivedAt":"2007-12-09T19:24:37Z","isPatch":true,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"\nOn Dec 9, 2007, at 1:15 PM, Junio C Hamano wrote:\n\n> The fix may be correct but the comment above the part the patch  \n> fixes is\n> very misleading and I wasted a few minutes checking to see what it is\n> doing is correct.\n\nThe fix is correct.  I did not anticipate empty strings when I wrote  \nthe loop in question.  Although I wonder if that should be expanded to  \nuse isspace instead of checking for just ' '.  I'm less familiar with  \nthe config parsing than I was.\n\n> The \"quote\" variable does not really control quoting; the quoting with\n> backslash is always done for the value, and this check is about  \n> adding a\n> dq pair around it, so that the parser does not lose leading or  \n> trailing\n> SP or string after start-of-comment marker characters.\n\nIt's for adding quotes, which made the comment make sense to me.  But  \nyour expanded description is correct and matches the commit message I  \nwrote.  Thanks for clarifying it in the code.\n\n~~ Brian\n"}]}