# git-grep Bus Error 9 messages from 2009-03-08 to 2009-03-09. Participants: Brian Gernhardt, Sam Hocevar, Junio C Hamano. Thread: https://gitlist.dev/t/18217 ## Brian Gernhardt, 2009-03-08 23:27 Subject: git-grep Bus Error Message-ID: URL: https://gitlist.dev/e/C36B091A-ABE9-4C74-9E59-4EBD50E3B9F5%40gernhardtsoftware.com ``` The --color display code in git-grep is giving me a bus error in show_line at line 492: > 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); The first problem is that %.*s does not appear to do on OS X what the author thinks it does. A precision of 0 for %s is listed in "man printf" as printing the entire string. To fix that, I changed it to the following: > if( match.rm_so > 0 ) > printf( "%.*s", match.rm_so, bol ); > if( match.rm_eo > match.rm_so ) > printf("%s%.*s%s", > opt->color_match, > match.rm_eo - match.rm_so, bol + match.rm_so, > GIT_COLOR_RESET); This code does not fail, but instead gives lines like the following (showing the raw color codes): .gitignore:\033[31m\033[1m(nugit GIT_COLOR_RESET is apparently being ignored, and I don't know why. Adding a line to check the values of rm_so, rm_eo, and the difference between the two gives: > printf( "%d %d %d", > match.rm_so, match.rm_eo, > match.rm_eo - match.rm_so ); .gitignore:0 0 3\033[31m\033[1m(nugit .mailmap:23 0 26(null)\033[31m\033[1m(nugit-shortlog to fix a few botched name translations-shortlog to fix a few botched name translations And now I'm baffled. Apparently my computer thinks 0 - 0 == 3 and 0 - 23 == 26. Can I get some help? ~~ Brian ``` ## Sam Hocevar, 2009-03-08 23:41 Subject: Re: git-grep Bus Error Message-ID: <20090308234141.GJ12880@zoy.org> URL: https://gitlist.dev/e/20090308234141.GJ12880%40zoy.org In-Reply-To: ``` On Sun, Mar 08, 2009, Brian Gernhardt wrote: > > printf( "%d %d %d", > > match.rm_so, match.rm_eo, > > match.rm_eo - match.rm_so ); > > .gitignore:0 0 3\033[31m\033[1m(nugit > .mailmap:23 0 26(null)\033[31m\033[1m(nugit-shortlog to fix a few > botched name translations-shortlog to fix a few botched name > translations > > And now I'm baffled. Apparently my computer thinks 0 - 0 == 3 and 0 - > 23 == 26. rm_so and rm_eo are ints on Linux but off_t's on Darwin, hence probably int64_t's here. You should cast the arguments. -- Sam. ``` ## Junio C Hamano, 2009-03-09 00:29 Subject: Re: git-grep Bus Error Message-ID: <7v1vt7k07o.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v1vt7k07o.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Brian Gernhardt writes: > The --color display code in git-grep is giving me a bus error in > show_line at line 492: > >> 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); > > The first problem is that %.*s does not appear to do on OS X what the > author thinks it does. Hmm, that means that printf on OSX does not dohat the POSIX thinks it ought to. http://www.opengroup.org/onlinepubs/009695399/functions/fprintf.html says that "a negative precision is taken as if the precision were omitted"; it does not say "a negative or zero" here. Which is a bit sad, because we would need to apply a workaround like yours. We shouldn't have to. > To fix that, I changed it to the following: > >> if( match.rm_so > 0 ) >> printf( "%.*s", match.rm_so, bol ); >> if( match.rm_eo > match.rm_so ) >> printf("%s%.*s%s", >> opt->color_match, >> match.rm_eo - match.rm_so, bol + match.rm_so, >> GIT_COLOR_RESET); > This code does not fail, but instead gives lines like the following > (showing the raw color codes): > > .gitignore:\033[31m\033[1m(nugit Hmm, that is strange. Your above change issues color_match and COLOR_RESET only when you have something between rm_eo and rm_so. I do not see anything between "ESC [ 31 m" and "ESC [ m" above, and you have an extra "1" between "ESC [" and terminating "m" in the reset sequence. ``` ## Brian Gernhardt, 2009-03-09 00:58 Subject: Re: git-grep Bus Error Message-ID: <49C11A48-5246-4477-9F33-26942B8C99D9@silverinsanity.com> URL: https://gitlist.dev/e/49C11A48-5246-4477-9F33-26942B8C99D9%40silverinsanity.com In-Reply-To: <20090308234141.GJ12880@zoy.org> ``` On Mar 8, 2009, at 7:41 PM, Sam Hocevar wrote: > On Sun, Mar 08, 2009, Brian Gernhardt wrote: > >>> printf( "%d %d %d", >>> match.rm_so, match.rm_eo, >>> match.rm_eo - match.rm_so ); >> >> .gitignore:0 0 3\033[31m\033[1m(nugit >> .mailmap:23 0 26(null)\033[31m\033[1m(nugit-shortlog to fix a few >> botched name translations-shortlog to fix a few botched name >> translations >> >> And now I'm baffled. Apparently my computer thinks 0 - 0 == 3 and >> 0 - >> 23 == 26. > > rm_so and rm_eo are ints on Linux but off_t's on Darwin, hence > probably int64_t's here. You should cast the arguments. And that explains the warnings about the parameters to printf not being integers. I was looking at compat/regex/regex.h and was confused. Adding a cast to int on all of the format specifiers solves my problems. Thank you. ~~ Brian ``` ## Junio C Hamano, 2009-03-09 01:11 Subject: Re: git-grep Bus Error Message-ID: <7vtz63ijoz.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vtz63ijoz.fsf%40gitster.siamese.dyndns.org In-Reply-To: <20090308234141.GJ12880@zoy.org> ``` Sam Hocevar writes: > On Sun, Mar 08, 2009, Brian Gernhardt wrote: > >> > printf( "%d %d %d", >> > match.rm_so, match.rm_eo, >> > match.rm_eo - match.rm_so ); >> >> .gitignore:0 0 3\033[31m\033[1m(nugit >> .mailmap:23 0 26(null)\033[31m\033[1m(nugit-shortlog to fix a few >> botched name translations-shortlog to fix a few botched name >> translations >> >> And now I'm baffled. Apparently my computer thinks 0 - 0 == 3 and 0 - >> 23 == 26. > > rm_so and rm_eo are ints on Linux but off_t's on Darwin, hence > probably int64_t's here. You should cast the arguments. That is a very good point. In fact, "git grep -n -e 'printf.*%\.\*s'" reveals that many existing call sites to this form casts the precision argument explicitly to "int". Brian, would this patch help? grep.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/grep.c b/grep.c index cace1c8..dcdbd5e 100644 --- a/grep.c +++ b/grep.c @@ -490,9 +490,9 @@ 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, + (int) match.rm_so, bol, opt->color_match, - match.rm_eo - match.rm_so, bol + match.rm_so, + (int)(match.rm_eo - match.rm_so), bol + match.rm_so, GIT_COLOR_RESET); bol += match.rm_eo; rest -= match.rm_eo; ``` ## Junio C Hamano, 2009-03-09 01:18 Subject: Re: git-grep Bus Error Message-ID: <7vprgrijd5.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vprgrijd5.fsf%40gitster.siamese.dyndns.org In-Reply-To: <7vtz63ijoz.fsf@gitster.siamese.dyndns.org> ``` Junio C Hamano writes: > Brian, would this patch help? > > grep.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/grep.c b/grep.c > index cace1c8..dcdbd5e 100644 > --- a/grep.c > +++ b/grep.c > @@ -490,9 +490,9 @@ 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, > + (int) match.rm_so, bol, > opt->color_match, > - match.rm_eo - match.rm_so, bol + match.rm_so, > + (int)(match.rm_eo - match.rm_so), bol + match.rm_so, > GIT_COLOR_RESET); > bol += match.rm_eo; > rest -= match.rm_eo; I looked at all the hits from $ git grep -n -e 'printf.*%\.\*s' --and --not -e '(int)' The above should be the only two places that need fixing. ``` ## Brian Gernhardt, 2009-03-09 01:19 Subject: Re: git-grep Bus Error Message-ID: <5A3896E0-E4F2-4AD1-8106-1E2CC82D18F5@gernhardtsoftware.com> URL: https://gitlist.dev/e/5A3896E0-E4F2-4AD1-8106-1E2CC82D18F5%40gernhardtsoftware.com In-Reply-To: <7vtz63ijoz.fsf@gitster.siamese.dyndns.org> ``` On Mar 8, 2009, at 9:11 PM, Junio C Hamano wrote: > Sam Hocevar writes: > >> rm_so and rm_eo are ints on Linux but off_t's on Darwin, hence >> probably int64_t's here. You should cast the arguments. > > That is a very good point. In fact, "git grep -n -e 'printf.*%\.\*s'" > reveals that many existing call sites to this form casts the precision > argument explicitly to "int". > > Brian, would this patch help? Yes, except that the code also depends on printf("%.*s", 0, str) working properly. I just sent a patch which checks for zero width and performs the casts. ~~ Brian ``` ## Junio C Hamano, 2009-03-09 01:24 Subject: [PATCH] grep: cast printf %.*s "precision" argument explicitly to int Message-ID: <7vljrfij3k.fsf_-_@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vljrfij3k.fsf_-_%40gitster.siamese.dyndns.org In-Reply-To: <7vtz63ijoz.fsf@gitster.siamese.dyndns.org> ``` On some systems, regoff_t that is the type of rm_so/rm_eo members are wider than int; %.*s precision specifier expects an int, so use an explicit cast. A breakage reported on Darwin by Brian Gernhardt should be fixed with this patch. Signed-off-by: Junio C Hamano --- Junio C Hamano writes: > Brian, would this patch help? A resend with a commit log message. grep.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/grep.c b/grep.c index cace1c8..be99b34 100644 --- a/grep.c +++ b/grep.c @@ -490,9 +490,9 @@ 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, + (int)match.rm_so, bol, opt->color_match, - match.rm_eo - match.rm_so, bol + match.rm_so, + (int)(match.rm_eo - match.rm_so), bol + match.rm_so, GIT_COLOR_RESET); bol += match.rm_eo; rest -= match.rm_eo; -- 1.6.2.206.g5bda76 ``` ## Brian Gernhardt, 2009-03-09 01:48 Subject: Re: git-grep Bus Error Message-ID: <981DB931-6FE4-484E-B101-EFCCAA5E2973@gernhardtsoftware.com> URL: https://gitlist.dev/e/981DB931-6FE4-484E-B101-EFCCAA5E2973%40gernhardtsoftware.com In-Reply-To: <7vtz63ijoz.fsf@gitster.siamese.dyndns.org> ``` On Mar 8, 2009, at 9:11 PM, Junio C Hamano wrote: > Brian, would this patch help? > > grep.c | 4 ++-- > 1 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/grep.c b/grep.c > index cace1c8..dcdbd5e 100644 > --- a/grep.c > +++ b/grep.c > @@ -490,9 +490,9 @@ 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, > + (int) match.rm_so, bol, > opt->color_match, > - match.rm_eo - match.rm_so, bol + match.rm_so, > + (int)(match.rm_eo - match.rm_so), bol + match.rm_so, > GIT_COLOR_RESET); > bol += match.rm_eo; > rest -= match.rm_eo; Apparently so. Despite the fact that match.rm_so is 0 at times, "%.*s" works properly so the other half of the patch isn't needed. Odd. ~~ B ```