Re: git-grep Bus Error

Sam Hocevar [off-list ref] writes:

On Sun, Mar 08, 2009, Brian Gernhardt wrote:

quoted

quoted

		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;

On Mar 8, 2009, at 9:11 PM, Junio C Hamano wrote:

Sam Hocevar [off-list ref] writes:

quoted

  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

On Mar 8, 2009, at 9:11 PM, Junio C Hamano wrote:

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