[PATCH] grep: make show_line more portable

[PATCH] grep: make show_line more portable

[Brian Gernhardt]

On OS X the printf specifier "%.0s" outputs the entire string instead
of 0 characters as POSIX states.

In addition, for * width or precision printf expects an integer
argument.  On systems were regoff_t is 64-bit, unexpected results can
occur.

To fix these, use if statements to catch 0 precisions and casts to
convert regoff_t to int.

Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---
 grep.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/grep.c b/grep.c
index cace1c8..ec68200 100644
--- a/grep.c
+++ b/grep.c
@@ -489,18 +489,22 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 
 		*eol = '\0';
 		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
-			printf("%.*s%s%.*s%s",
-			       match.rm_so, bol,
-			       opt->color_match,
-			       match.rm_eo - match.rm_so, bol + match.rm_so,
-			       GIT_COLOR_RESET);
+			if( match.rm_so > 0 )
+				printf( "%.*s", (int) match.rm_so, bol );
+			if( match.rm_eo > match.rm_so )
+				printf("%s%.*s%s",
+					   opt->color_match,
+					  (int) (match.rm_eo - match.rm_so), bol + match.rm_so,
+					   GIT_COLOR_RESET);
 			bol += match.rm_eo;
 			rest -= match.rm_eo;
 			eflags = REG_NOTBOL;
 		}
 		*eol = ch;
 	}
-	printf("%.*s\n", rest, bol);
+	if( rest > 0 )
+		printf("%.*s", rest, bol);
+	printf("\n");
 }
 
 static int grep_buffer_1(struct grep_opt *opt, const char *name,
-- 
1.6.2.222.g01cbd

Re: [PATCH] grep: make show_line more portable

[Junio C Hamano]

Brian Gernhardt <benji@silverinsanity.com> writes:

> On OS X the printf specifier "%.0s" outputs the entire string instead
> of 0 characters as POSIX states.
>
> In addition, for * width or precision printf expects an integer
> argument.  On systems were regoff_t is 64-bit, unexpected results can
> occur.

I would prefer to see these two issues solved as separate issues.

Specifically, I'd like to know if the patch from me to you a few message
ago solves the issue.

If you still need a "some implementations of printf is broken with respect
to 0 precision" workaround on top of that patch, we would want to add it
separately, but it may have to cover not just this printf(), as I am not
convinced this is the only place that lets (integer) 0 passed to the
"%.*s" format.  That patch needs to be written after a separate auditing
of output from "git grep -n -e 'printf.*%\.\*s'", which I do not think
happened yet (at least I haven't done that, and I somehow do not think you
have yet either).

Re: [PATCH] grep: make show_line more portable

[Jay Soffian]

On Sun, Mar 8, 2009 at 9:15 PM, Brian Gernhardt
<benji@silverinsanity.com> wrote:
> On OS X the printf specifier "%.0s" outputs the entire string instead
> of 0 characters as POSIX states.

Does not reproduce for me:

$ cat foo.c && gcc -m64 foo.c -o foo32 && gcc foo.c -o foo64 && file
foo32 foo64 && ./foo32 && ./foo64
#include "stdio.h"
#include "stdlib.h"
main() {
	printf("1 '%.0s'\n", "foobar");
	printf("2 '%.*s'\n", 0, "foobar");
	exit(0);
}
foo32: Mach-O 64-bit executable x86_64
foo64: Mach-O executable i386
1 ''
2 ''
1 ''
2 ''

OS X 10.5.6 (Darwin 9.6.0). i686-apple-darwin9-gcc-4.0.1. Same linkage for both:

/usr/lib/libgcc_s.1.dylib (compatibility version 1.0.0, current version 1.0.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current
version 111.1.3)

j.

Re: [PATCH] grep: make show_line more portable

[Jay Soffian]

On Sun, Mar 8, 2009 at 10:22 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> foo32: Mach-O 64-bit executable x86_64
> foo64: Mach-O executable i386

Okay, so I may be brain-damaged in my naming, but that doesn't
invalidate the results. :-)

j.

Re: [PATCH] grep: make show_line more portable

[Brian Gernhardt]

On Mar 8, 2009, at 10:22 PM, Jay Soffian wrote:

> On Sun, Mar 8, 2009 at 9:15 PM, Brian Gernhardt
> <benji@silverinsanity.com> wrote:
>> On OS X the printf specifier "%.0s" outputs the entire string instead
>> of 0 characters as POSIX states.
>
> Does not reproduce for me:

Nor for me, as I noted on the other thread...  And looking again, I  
was reading the man page for printf(1), not printf(3).  Ouch.   
*grumble, grumble*  I'm crawling back under my rock now.

~~ B

Re: [PATCH] grep: make show_line more portable

[Junio C Hamano]

Brian Gernhardt <benji@silverinsanity.com> writes:

> On Mar 8, 2009, at 10:22 PM, Jay Soffian wrote:
>
>> On Sun, Mar 8, 2009 at 9:15 PM, Brian Gernhardt
>> <benji@silverinsanity.com> wrote:
>>> On OS X the printf specifier "%.0s" outputs the entire string instead
>>> of 0 characters as POSIX states.
>>
>> Does not reproduce for me:
>
> Nor for me, as I noted on the other thread...  And looking again, I
> was reading the man page for printf(1), not printf(3).  Ouch.
> *grumble, grumble*  I'm crawling back under my rock now.

Heh, people make mistakes and others are here to help spot them.
Collectively we all win.

Thanks for a breakage report, initial fix and a confirmation.

Re: [PATCH] grep: make show_line more portable

[Johannes Schindelin]

Hi,

On Sun, 8 Mar 2009, Junio C Hamano wrote:

> Brian Gernhardt <benji@silverinsanity.com> writes:
> 
> > On Mar 8, 2009, at 10:22 PM, Jay Soffian wrote:
> >
> >> On Sun, Mar 8, 2009 at 9:15 PM, Brian Gernhardt
> >> <benji@silverinsanity.com> wrote:
> >>> On OS X the printf specifier "%.0s" outputs the entire string instead
> >>> of 0 characters as POSIX states.
> >>
> >> Does not reproduce for me:
> >
> > Nor for me, as I noted on the other thread...  And looking again, I
> > was reading the man page for printf(1), not printf(3).  Ouch.
> > *grumble, grumble*  I'm crawling back under my rock now.
> 
> Heh, people make mistakes and others are here to help spot them.
> Collectively we all win.

One of my favorite quotes these days:

The computer "doth make fools of us all," so that any fool without the 
ability to share a laugh on himself will be unable to tolerate programming 
for long. ''(Gerald M. Weinberg)''

> Thanks for a breakage report, initial fix and a confirmation.

Yes, I think this discussion was valuable, not only because it fixed a 
bug, but also because I learnt that %.*s with a negative length defaults 
to the total string.

Ciao,
Dscho

Re: [PATCH] grep: make show_line more portable

[René Scharfe]

Brian Gernhardt schrieb:
> 
> On Mar 8, 2009, at 10:22 PM, Jay Soffian wrote:
> 
>> On Sun, Mar 8, 2009 at 9:15 PM, Brian Gernhardt
>> <benji@silverinsanity.com> wrote:
>>> On OS X the printf specifier "%.0s" outputs the entire string instead
>>> of 0 characters as POSIX states.
>>
>> Does not reproduce for me:
> 
> Nor for me, as I noted on the other thread...  And looking again, I was
> reading the man page for printf(1), not printf(3).  Ouch.  *grumble,
> grumble*  I'm crawling back under my rock now.

Sorry for introducing a Linuxism. :-/  Thanks for testing and reporting
and for fixing the bug.

René