# [PATCH] grep: make show_line more portable 8 messages from 2009-03-09 to 2009-03-09. Participants: Brian Gernhardt, Junio C Hamano, Jay Soffian, Johannes Schindelin, René Scharfe. Thread: https://gitlist.dev/t/18221 ## Brian Gernhardt, 2009-03-09 01:15 Subject: [PATCH] grep: make show_line more portable Message-ID: <1236561326-1231-1-git-send-email-benji@silverinsanity.com> URL: https://gitlist.dev/e/1236561326-1231-1-git-send-email-benji%40silverinsanity.com ``` 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 --- 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 ``` ## Junio C Hamano, 2009-03-09 01:35 Subject: Re: [PATCH] grep: make show_line more portable Message-ID: <7vhc23iil4.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vhc23iil4.fsf%40gitster.siamese.dyndns.org In-Reply-To: <1236561326-1231-1-git-send-email-benji@silverinsanity.com> ``` Brian Gernhardt 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). ``` ## Jay Soffian, 2009-03-09 02:22 Subject: Re: [PATCH] grep: make show_line more portable Message-ID: <76718490903081922p105ebf79vb0bf06989413887c@mail.gmail.com> URL: https://gitlist.dev/e/76718490903081922p105ebf79vb0bf06989413887c%40mail.gmail.com In-Reply-To: <1236561326-1231-1-git-send-email-benji@silverinsanity.com> ``` On Sun, Mar 8, 2009 at 9:15 PM, Brian Gernhardt 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. ``` ## Jay Soffian, 2009-03-09 02:23 Subject: Re: [PATCH] grep: make show_line more portable Message-ID: <76718490903081923w3779d6aeu4081022a6630eb73@mail.gmail.com> URL: https://gitlist.dev/e/76718490903081923w3779d6aeu4081022a6630eb73%40mail.gmail.com In-Reply-To: <76718490903081922p105ebf79vb0bf06989413887c@mail.gmail.com> ``` On Sun, Mar 8, 2009 at 10:22 PM, Jay Soffian 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. ``` ## Brian Gernhardt, 2009-03-09 02:44 Subject: Re: [PATCH] grep: make show_line more portable Message-ID: <5ACAF49A-84B5-4F55-A8B8-0FC711708810@silverinsanity.com> URL: https://gitlist.dev/e/5ACAF49A-84B5-4F55-A8B8-0FC711708810%40silverinsanity.com In-Reply-To: <76718490903081922p105ebf79vb0bf06989413887c@mail.gmail.com> ``` On Mar 8, 2009, at 10:22 PM, Jay Soffian wrote: > On Sun, Mar 8, 2009 at 9:15 PM, Brian Gernhardt > 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 ``` ## Junio C Hamano, 2009-03-09 03:52 Subject: Re: [PATCH] grep: make show_line more portable Message-ID: <7vbpsbic85.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vbpsbic85.fsf%40gitster.siamese.dyndns.org In-Reply-To: <5ACAF49A-84B5-4F55-A8B8-0FC711708810@silverinsanity.com> ``` Brian Gernhardt writes: > On Mar 8, 2009, at 10:22 PM, Jay Soffian wrote: > >> On Sun, Mar 8, 2009 at 9:15 PM, Brian Gernhardt >> 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. ``` ## Johannes Schindelin, 2009-03-09 09:50 Subject: Re: [PATCH] grep: make show_line more portable Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0903091049150.10279%40pacific.mpi-cbg.de In-Reply-To: <7vbpsbic85.fsf@gitster.siamese.dyndns.org> ``` Hi, On Sun, 8 Mar 2009, Junio C Hamano wrote: > Brian Gernhardt writes: > > > On Mar 8, 2009, at 10:22 PM, Jay Soffian wrote: > > > >> On Sun, Mar 8, 2009 at 9:15 PM, Brian Gernhardt > >> 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 ``` ## René Scharfe, 2009-03-09 19:34 Subject: Re: [PATCH] grep: make show_line more portable Message-ID: <49B56F2B.1080402@lsrfire.ath.cx> URL: https://gitlist.dev/e/49B56F2B.1080402%40lsrfire.ath.cx In-Reply-To: <5ACAF49A-84B5-4F55-A8B8-0FC711708810@silverinsanity.com> ``` 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 >> 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é ```