{"thread":{"id":"18217","subject":"git-grep Bus Error","startedAt":"2009-03-08T23:27:01Z","lastAt":"2009-03-09T01:48:02Z","messageCount":9,"participants":["Brian Gernhardt","Sam Hocevar","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"107413","messageId":"C36B091A-ABE9-4C74-9E59-4EBD50E3B9F5@gernhardtsoftware.com","threadId":"18217","inReplyTo":null,"subject":"git-grep Bus Error","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2009-03-08T23:27:01Z","receivedAt":"2009-03-08T23:27:01Z","isPatch":false,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"The --color display code in git-grep is giving me a bus error in \nshow_line at line 492:\n\n> printf(\"%.*s%s%.*s%s\",\n> match.rm_so, bol,\n> opt->color_match,\n> match.rm_eo - match.rm_so, bol + \n> match.rm_so,\n> GIT_COLOR_RESET);\n\nThe first problem is that %.*s does not appear to do on OS X what the \nauthor thinks it does. A precision of 0 for %s is listed in \"man \nprintf\" as printing the entire string.\n\nTo fix that, I changed it to the following:\n\n> \t\t\tif( match.rm_so > 0 )\n> \t\t\t\tprintf( \"%.*s\", match.rm_so, bol );\n> \t\t\tif( match.rm_eo > match.rm_so )\n> \t\t\t\tprintf(\"%s%.*s%s\",\n> \t\t\t\t\t opt->color_match,\n> \t\t\t\t\t match.rm_eo - match.rm_so, bol + match.rm_so,\n> \t\t\t\t\t GIT_COLOR_RESET);\n\nThis code does not fail, but instead gives lines like the following \n(showing the raw color codes):\n\n.gitignore:\\033[31m\\033[1m(nugit\n\nGIT_COLOR_RESET is apparently being ignored, and I don't know why.\n\nAdding a line to check the values of rm_so, rm_eo, and the difference \nbetween the two gives:\n\n> \t\t\tprintf( \"%d %d %d\",\n> \t\t\t\t match.rm_so, match.rm_eo,\n> \t\t\t\t match.rm_eo - match.rm_so );\n\n.gitignore:0 0 3\\033[31m\\033[1m(nugit\n.mailmap:23 0 26(null)\\033[31m\\033[1m(nugit-shortlog to fix a few \nbotched name translations-shortlog to fix a few botched name \ntranslations\n\nAnd now I'm baffled. Apparently my computer thinks 0 - 0 == 3 and 0 - \n23 == 26.\n\nCan I get some help?\n\n~~ Brian\n"},{"id":"107414","messageId":"20090308234141.GJ12880@zoy.org","threadId":"18217","inReplyTo":"C36B091A-ABE9-4C74-9E59-4EBD50E3B9F5@gernhardtsoftware.com","subject":"Re: git-grep Bus Error","fromName":"Sam Hocevar","fromEmail":"sam@zoy.org","sentAt":"2009-03-08T23:41:41Z","receivedAt":"2009-03-08T23:41:41Z","isPatch":false,"sender":{"key":"sam@zoy.org","avatar":"https://gravatar.com/avatar/1fc1e5d8c3a8d737f14572135671adfbc0e61ffba5c3bc1d1b8a6f4aac764470?d=mp&s=160"},"body":"On Sun, Mar 08, 2009, Brian Gernhardt wrote:\n\n> >\t\t\tprintf( \"%d %d %d\",\n> >\t\t\t\t match.rm_so, match.rm_eo,\n> >\t\t\t\t match.rm_eo - match.rm_so );\n> \n> .gitignore:0 0 3\\033[31m\\033[1m(nugit\n> .mailmap:23 0 26(null)\\033[31m\\033[1m(nugit-shortlog to fix a few \n> botched name translations-shortlog to fix a few botched name \n> translations\n> \n> And now I'm baffled. Apparently my computer thinks 0 - 0 == 3 and 0 - \n> 23 == 26.\n\n rm_so and rm_eo are ints on Linux but off_t's on Darwin, hence\nprobably int64_t's here. You should cast the arguments.\n\n-- \nSam.\n"},{"id":"107416","messageId":"7v1vt7k07o.fsf@gitster.siamese.dyndns.org","threadId":"18217","inReplyTo":"C36B091A-ABE9-4C74-9E59-4EBD50E3B9F5@gernhardtsoftware.com","subject":"Re: git-grep Bus Error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-09T00:29:31Z","receivedAt":"2009-03-09T00:29:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brian Gernhardt writes:\n\n> The --color display code in git-grep is giving me a bus error in\n> show_line at line 492:\n>\n>> printf(\"%.*s%s%.*s%s\",\n>> match.rm_so, bol,\n>> opt->color_match,\n>> match.rm_eo - match.rm_so, bol +\n>> match.rm_so,\n>> GIT_COLOR_RESET);\n>\n> The first problem is that %.*s does not appear to do on OS X what the\n> author thinks it does.\n\nHmm, that means that printf on OSX does not dohat the POSIX thinks it\nought to.\n\n http://www.opengroup.org/onlinepubs/009695399/functions/fprintf.html\n\nsays that \"a negative precision is taken as if the precision were\nomitted\"; it does not say \"a negative or zero\" here.\n\nWhich is a bit sad, because we would need to apply a workaround like\nyours. We shouldn't have to.\n\n> To fix that, I changed it to the following:\n>\n>> \t\t\tif( match.rm_so > 0 )\n>> \t\t\t\tprintf( \"%.*s\", match.rm_so, bol );\n>> \t\t\tif( match.rm_eo > match.rm_so )\n>> \t\t\t\tprintf(\"%s%.*s%s\",\n>> \t\t\t\t\t opt->color_match,\n>> \t\t\t\t\t match.rm_eo - match.rm_so, bol + match.rm_so,\n>> \t\t\t\t\t GIT_COLOR_RESET);\n\n> This code does not fail, but instead gives lines like the following\n> (showing the raw color codes):\n>\n> .gitignore:\\033[31m\\033[1m(nugit\n\nHmm, that is strange. Your above change issues color_match and COLOR_RESET\nonly when you have something between rm_eo and rm_so. I do not see\nanything between \"ESC [ 31 m\" and \"ESC [ m\" above, and you have an extra \"1\"\nbetween \"ESC [\" and terminating \"m\" in the reset sequence.\n"},{"id":"107417","messageId":"49C11A48-5246-4477-9F33-26942B8C99D9@silverinsanity.com","threadId":"18217","inReplyTo":"20090308234141.GJ12880@zoy.org","subject":"Re: git-grep Bus Error","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2009-03-09T00:58:42Z","receivedAt":"2009-03-09T00:58:42Z","isPatch":false,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"\nOn Mar 8, 2009, at 7:41 PM, Sam Hocevar wrote:\n\n> On Sun, Mar 08, 2009, Brian Gernhardt wrote:\n>\n>>> \t\t\tprintf( \"%d %d %d\",\n>>> \t\t\t\t match.rm_so, match.rm_eo,\n>>> \t\t\t\t match.rm_eo - match.rm_so );\n>>\n>> .gitignore:0 0 3\\033[31m\\033[1m(nugit\n>> .mailmap:23 0 26(null)\\033[31m\\033[1m(nugit-shortlog to fix a few\n>> botched name translations-shortlog to fix a few botched name\n>> translations\n>>\n>> And now I'm baffled. Apparently my computer thinks 0 - 0 == 3 and \n>> 0 -\n>> 23 == 26.\n>\n> rm_so and rm_eo are ints on Linux but off_t's on Darwin, hence\n> probably int64_t's here. You should cast the arguments.\n\n\nAnd that explains the warnings about the parameters to printf not \nbeing integers. I was looking at compat/regex/regex.h and was confused.\n\nAdding a cast to int on all of the format specifiers solves my \nproblems. Thank you.\n\n~~ Brian\n"},{"id":"107421","messageId":"7vtz63ijoz.fsf@gitster.siamese.dyndns.org","threadId":"18217","inReplyTo":"20090308234141.GJ12880@zoy.org","subject":"Re: git-grep Bus Error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-09T01:11:40Z","receivedAt":"2009-03-09T01:11:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sam Hocevar writes:\n\n> On Sun, Mar 08, 2009, Brian Gernhardt wrote:\n>\n>> >\t\t\tprintf( \"%d %d %d\",\n>> >\t\t\t\t match.rm_so, match.rm_eo,\n>> >\t\t\t\t match.rm_eo - match.rm_so );\n>> \n>> .gitignore:0 0 3\\033[31m\\033[1m(nugit\n>> .mailmap:23 0 26(null)\\033[31m\\033[1m(nugit-shortlog to fix a few \n>> botched name translations-shortlog to fix a few botched name \n>> translations\n>> \n>> And now I'm baffled. Apparently my computer thinks 0 - 0 == 3 and 0 - \n>> 23 == 26.\n>\n> rm_so and rm_eo are ints on Linux but off_t's on Darwin, hence\n> probably int64_t's here. You should cast the arguments.\n\nThat is a very good point. In fact, \"git grep -n -e 'printf.*%\\.\\*s'\"\nreveals that many existing call sites to this form casts the precision\nargument explicitly to \"int\".\n\nBrian, would this patch help?\n\n grep.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex cace1c8..dcdbd5e 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -490,9 +490,9 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\t*eol = '\\0';\n \t\twhile (next_match(opt, bol, eol, ctx, &match, eflags)) {\n \t\t\tprintf(\"%.*s%s%.*s%s\",\n-\t\t\t match.rm_so, bol,\n+\t\t\t (int) match.rm_so, bol,\n \t\t\t opt->color_match,\n-\t\t\t match.rm_eo - match.rm_so, bol + match.rm_so,\n+\t\t\t (int)(match.rm_eo - match.rm_so), bol + match.rm_so,\n \t\t\t GIT_COLOR_RESET);\n \t\t\tbol += match.rm_eo;\n \t\t\trest -= match.rm_eo;\n"},{"id":"107424","messageId":"7vprgrijd5.fsf@gitster.siamese.dyndns.org","threadId":"18217","inReplyTo":"7vtz63ijoz.fsf@gitster.siamese.dyndns.org","subject":"Re: git-grep Bus Error","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-09T01:18:46Z","receivedAt":"2009-03-09T01:18:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Brian, would this patch help?\n>\n> grep.c | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/grep.c b/grep.c\n> index cace1c8..dcdbd5e 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -490,9 +490,9 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n> \t\t*eol = '\\0';\n> \t\twhile (next_match(opt, bol, eol, ctx, &match, eflags)) {\n> \t\t\tprintf(\"%.*s%s%.*s%s\",\n> -\t\t\t match.rm_so, bol,\n> +\t\t\t (int) match.rm_so, bol,\n> \t\t\t opt->color_match,\n> -\t\t\t match.rm_eo - match.rm_so, bol + match.rm_so,\n> +\t\t\t (int)(match.rm_eo - match.rm_so), bol + match.rm_so,\n> \t\t\t GIT_COLOR_RESET);\n> \t\t\tbol += match.rm_eo;\n> \t\t\trest -= match.rm_eo;\n\nI looked at all the hits from\n\n $ git grep -n -e 'printf.*%\\.\\*s' --and --not -e '(int)'\n\nThe above should be the only two places that need fixing.\n"},{"id":"107425","messageId":"5A3896E0-E4F2-4AD1-8106-1E2CC82D18F5@gernhardtsoftware.com","threadId":"18217","inReplyTo":"7vtz63ijoz.fsf@gitster.siamese.dyndns.org","subject":"Re: git-grep Bus Error","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2009-03-09T01:19:35Z","receivedAt":"2009-03-09T01:19:35Z","isPatch":false,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"\nOn Mar 8, 2009, at 9:11 PM, Junio C Hamano wrote:\n\n> Sam Hocevar writes:\n>\n>> rm_so and rm_eo are ints on Linux but off_t's on Darwin, hence\n>> probably int64_t's here. You should cast the arguments.\n>\n> That is a very good point. In fact, \"git grep -n -e 'printf.*%\\.\\*s'\"\n> reveals that many existing call sites to this form casts the precision\n> argument explicitly to \"int\".\n>\n> Brian, would this patch help?\n\nYes, except that the code also depends on printf(\"%.*s\", 0, str) \nworking properly. I just sent a patch which checks for zero width and \nperforms the casts.\n\n~~ Brian\n"},{"id":"107426","messageId":"7vljrfij3k.fsf_-_@gitster.siamese.dyndns.org","threadId":"18217","inReplyTo":"7vtz63ijoz.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] grep: cast printf %.*s \"precision\" argument explicitly to int","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-09T01:24:31Z","receivedAt":"2009-03-09T01:24:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On some systems, regoff_t that is the type of rm_so/rm_eo members are\nwider than int; %.*s precision specifier expects an int, so use an explicit\ncast.\n\nA breakage reported on Darwin by Brian Gernhardt should be fixed with\nthis patch.\n\nSigned-off-by: Junio C Hamano \n---\n\n Junio C Hamano writes:\n\n > Brian, would this patch help?\n\n A resend with a commit log message.\n\n grep.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex cace1c8..be99b34 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -490,9 +490,9 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\t*eol = '\\0';\n \t\twhile (next_match(opt, bol, eol, ctx, &match, eflags)) {\n \t\t\tprintf(\"%.*s%s%.*s%s\",\n-\t\t\t match.rm_so, bol,\n+\t\t\t (int)match.rm_so, bol,\n \t\t\t opt->color_match,\n-\t\t\t match.rm_eo - match.rm_so, bol + match.rm_so,\n+\t\t\t (int)(match.rm_eo - match.rm_so), bol + match.rm_so,\n \t\t\t GIT_COLOR_RESET);\n \t\t\tbol += match.rm_eo;\n \t\t\trest -= match.rm_eo;\n-- \n1.6.2.206.g5bda76\n"},{"id":"107430","messageId":"981DB931-6FE4-484E-B101-EFCCAA5E2973@gernhardtsoftware.com","threadId":"18217","inReplyTo":"7vtz63ijoz.fsf@gitster.siamese.dyndns.org","subject":"Re: git-grep Bus Error","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2009-03-09T01:48:02Z","receivedAt":"2009-03-09T01:48:02Z","isPatch":false,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"\nOn Mar 8, 2009, at 9:11 PM, Junio C Hamano wrote:\n\n> Brian, would this patch help?\n>\n> grep.c | 4 ++--\n> 1 files changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/grep.c b/grep.c\n> index cace1c8..dcdbd5e 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -490,9 +490,9 @@ static void show_line(struct grep_opt *opt, char \n> *bol, char *eol,\n> \t\t*eol = '\\0';\n> \t\twhile (next_match(opt, bol, eol, ctx, &match, eflags)) {\n> \t\t\tprintf(\"%.*s%s%.*s%s\",\n> -\t\t\t match.rm_so, bol,\n> +\t\t\t (int) match.rm_so, bol,\n> \t\t\t opt->color_match,\n> -\t\t\t match.rm_eo - match.rm_so, bol + match.rm_so,\n> +\t\t\t (int)(match.rm_eo - match.rm_so), bol + match.rm_so,\n> \t\t\t GIT_COLOR_RESET);\n> \t\t\tbol += match.rm_eo;\n> \t\t\trest -= match.rm_eo;\n\nApparently so. Despite the fact that match.rm_so is 0 at times, \n\"%.*s\" works properly so the other half of the patch isn't needed. Odd.\n\n~~ B\n"}]}