# [PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer. 3 messages from 2007-12-08 to 2007-12-09. Participants: Jim Meyering, Junio C Hamano, Brian Gernhardt. Thread: https://gitlist.dev/t/11197 ## Jim Meyering, 2007-12-08 15:48 Subject: [PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer. Message-ID: <87hcitgpca.fsf@rho.meyering.net> URL: https://gitlist.dev/e/87hcitgpca.fsf%40rho.meyering.net ``` * config.c (store_write_pair): Don't read value[-1]. Signed-off-by: Jim Meyering --- config.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/config.c b/config.c index ed96213..6031b38 100644 --- a/config.c +++ b/config.c @@ -652,7 +652,7 @@ static int store_write_pair(int fd, const char* key, const char* value) for (i = 0; value[i]; i++) if (value[i] == ';' || value[i] == '#') quote = 1; - if (value[i-1] == ' ') + if (i && value[i-1] == ' ') quote = 1; if (write_in_full(fd, "\t", 1) != 1 || -- 1.5.3.7.1116.gae2a9 ``` ## Junio C Hamano, 2007-12-09 18:15 Subject: Re: [PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer. Message-ID: <7vsl2bpwe1.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vsl2bpwe1.fsf%40gitster.siamese.dyndns.org In-Reply-To: <87hcitgpca.fsf@rho.meyering.net> ``` Jim Meyering writes: > * config.c (store_write_pair): Don't read value[-1]. > > Signed-off-by: Jim Meyering > --- > config.c | 2 +- > 1 files changed, 1 insertions(+), 1 deletions(-) > > diff --git a/config.c b/config.c > index ed96213..6031b38 100644 > --- a/config.c > +++ b/config.c > @@ -652,7 +652,7 @@ static int store_write_pair(int fd, const char* key, const char* value) > for (i = 0; value[i]; i++) > if (value[i] == ';' || value[i] == '#') > quote = 1; > - if (value[i-1] == ' ') > + if (i && value[i-1] == ' ') > quote = 1; > > if (write_in_full(fd, "\t", 1) != 1 || Thanks. The fix may be correct but the comment above the part the patch fixes is very misleading and I wasted a few minutes checking to see what it is doing is correct. The "quote" variable does not really control quoting; the quoting with backslash is always done for the value, and this check is about adding a dq pair around it, so that the parser does not lose leading or trailing SP or string after start-of-comment marker characters. ``` ## Brian Gernhardt, 2007-12-09 19:24 Subject: Re: [PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer. Message-ID: <33716C48-068F-42C5-BD84-608EE9017636@silverinsanity.com> URL: https://gitlist.dev/e/33716C48-068F-42C5-BD84-608EE9017636%40silverinsanity.com In-Reply-To: <7vsl2bpwe1.fsf@gitster.siamese.dyndns.org> ``` On Dec 9, 2007, at 1:15 PM, Junio C Hamano wrote: > The fix may be correct but the comment above the part the patch > fixes is > very misleading and I wasted a few minutes checking to see what it is > doing is correct. The fix is correct. I did not anticipate empty strings when I wrote the loop in question. Although I wonder if that should be expanded to use isspace instead of checking for just ' '. I'm less familiar with the config parsing than I was. > The "quote" variable does not really control quoting; the quoting with > backslash is always done for the value, and this check is about > adding a > dq pair around it, so that the parser does not lose leading or > trailing > SP or string after start-of-comment marker characters. It's for adding quotes, which made the comment make sense to me. But your expanded description is correct and matches the commit message I wrote. Thanks for clarifying it in the code. ~~ Brian ```