threads / patch / 10820

patchfor-each-ref: fix off by one read.

Subject: [PATCH] for-each-ref: fix off by one read.

## tl;dr

3 messages between Nov 12, 2007 and Nov 13, 2007. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Christian Couder· Nov 12, 2007, 04:37 UTC · lore

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin-for-each-ref.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to builtin-for-each-ref.c +1 −1

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 598d4e1..89ea37c 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -306,7 +306,7 @@ static const char *find_wholine(const char *who, int wholen, const char *buf, un
 		if (!eol)
 			return "";
 		eol++;
-		if (eol[1] == '\n')
+		if (*eol == '\n')
 			return ""; /* end of header */
 		buf = eol;
 	}

-- 
1.5.3.5.577.g4d1b9-dirty

Junio C Hamano· Nov 13, 2007, 00:05 UTC · re: Christian Couder · lore

Re: [PATCH] for-each-ref: fix off by one read.

Christian Couder <chriscool@tuxfamily.org> writes:

Show 13 quoted lines

> diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
> index 598d4e1..89ea37c 100644
> --- a/builtin-for-each-ref.c
> +++ b/builtin-for-each-ref.c
> @@ -306,7 +306,7 @@ static const char *find_wholine(const char *who, int wholen, const char *buf, un
>  		if (!eol)
>  			return "";
>  		eol++;
> -		if (eol[1] == '\n')
> +		if (*eol == '\n')
>  			return ""; /* end of header */
>  		buf = eol;
>  	}

Good eyes.  This would have broken if

 (1) we had a header field that consists of a single character
     and then LF.  We would have mistaken such a line as the end
     of header; or

 (2) we had a commit or a tag that consists solely of header and
     no body.  We would have read past the terminating NUL.

Thanks.

Christian Couder· Nov 13, 2007, 05:15 UTC · re: Junio C Hamano · lore

Re: [PATCH] for-each-ref: fix off by one read.

Le mardi 13 novembre 2007, Junio C Hamano a écrit :

Show 16 quoted lines

> Christian Couder <chriscool@tuxfamily.org> writes:
> > diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
> > index 598d4e1..89ea37c 100644
> > --- a/builtin-for-each-ref.c
> > +++ b/builtin-for-each-ref.c
> > @@ -306,7 +306,7 @@ static const char *find_wholine(const char *who,
> > int wholen, const char *buf, un if (!eol)
> >  			return "";
> >  		eol++;
> > -		if (eol[1] == '\n')
> > +		if (*eol == '\n')
> >  			return ""; /* end of header */
> >  		buf = eol;
> >  	}
>
> Good eyes.

Well, Valgrind found it when I did:

$ valgrind git for-each-ref --format='%(refname)' "refs/tags/*"

> This would have broken if

Show 6 quoted lines

>  (1) we had a header field that consists of a single character
>      and then LF.  We would have mistaken such a line as the end
>      of header; or
>
>  (2) we had a commit or a tag that consists solely of header and
>      no body.  We would have read past the terminating NUL.

I suspect that the end of the header was not properly detected, so that it may have read one byte past a body ending with LF and the terminating NUL.

Christian.