{"thread":{"id":"44094","subject":"[PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","startedAt":"2016-09-15T18:31:42Z","lastAt":"2016-10-02T22:59:05Z","messageCount":10,"participants":["René Scharfe","Jeff King","Junio C Hamano","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"301994","messageId":"f7294ac5-8302-03fb-d756-81a1c029a813@web.de","threadId":"44094","inReplyTo":null,"subject":"[PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-09-15T18:31:00Z","receivedAt":"2016-09-15T18:31:42Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Replace uses of strbuf_addf() for adding strings with more lightweight\nstrbuf_addstr() calls.  This makes the intent clearer and avoids\npotential issues with printf format specifiers.\n\n02962d36845b89145cd69f8bc65e015d78ae3434 already converted six cases,\nthis patch covers eleven more.\n\nA semantic patch for Coccinelle is included for easier checking for\nnew cases that might be introduced in the future.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\nSilly question: Is there a natural language that uses percent signs\nas letters or e.g. instead of commas? :)\n\n builtin/fmt-merge-msg.c         | 2 +-\n builtin/merge.c                 | 2 +-\n builtin/submodule--helper.c     | 5 +++--\n contrib/coccinelle/strbuf.cocci | 5 +++++\n merge-recursive.c               | 2 +-\n remote.c                        | 8 ++++----\n wt-status.c                     | 6 +++---\n 7 files changed, 18 insertions(+), 12 deletions(-)\n create mode 100644 contrib/coccinelle/strbuf.cocci\n\ndiff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c\nindex ac84e99..dc2e9e4 100644\n--- a/builtin/fmt-merge-msg.c\n+++ b/builtin/fmt-merge-msg.c\n@@ -395,7 +395,7 @@ static void shortlog(const char *name,\n \n \tfor (i = 0; i < subjects.nr; i++)\n \t\tif (i >= limit)\n-\t\t\tstrbuf_addf(out, \"  ...\\n\");\n+\t\t\tstrbuf_addstr(out, \"  ...\\n\");\n \t\telse\n \t\t\tstrbuf_addf(out, \"  %s\\n\", subjects.items[i].string);\n \ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 0ae099f..a8b57c7 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -940,7 +940,7 @@ static void write_merge_state(struct commit_list *remoteheads)\n \n \tstrbuf_reset(&buf);\n \tif (fast_forward == FF_NO)\n-\t\tstrbuf_addf(&buf, \"no-ff\");\n+\t\tstrbuf_addstr(&buf, \"no-ff\");\n \twrite_file_buf(git_path_merge_mode(), buf.buf, buf.len);\n }\n \ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 9d79f19..ad23155 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -859,8 +859,9 @@ static int update_clone_get_next_task(struct child_process *child,\n \t\tce = suc->failed_clones[index];\n \t\tif (!prepare_to_clone_next_submodule(ce, child, suc, err)) {\n \t\t\tsuc->current ++;\n-\t\t\tstrbuf_addf(err, \"BUG: submodule considered for cloning,\"\n-\t\t\t\t    \"doesn't need cloning any more?\\n\");\n+\t\t\tstrbuf_addstr(err, \"BUG: submodule considered for \"\n+\t\t\t\t\t   \"cloning, doesn't need cloning \"\n+\t\t\t\t\t   \"any more?\\n\");\n \t\t\treturn 0;\n \t\t}\n \t\tp = xmalloc(sizeof(*p));\ndiff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci\nnew file mode 100644\nindex 0000000..7932d48\n--- /dev/null\n+++ b/contrib/coccinelle/strbuf.cocci\n@@ -0,0 +1,5 @@\n+@@\n+expression E1, E2;\n+@@\n+- strbuf_addf(E1, E2);\n++ strbuf_addstr(E1, E2);\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex e349126..d2b191b 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -206,7 +206,7 @@ static void output_commit_title(struct merge_options *o, struct commit *commit)\n \t\t\tfind_unique_abbrev(commit->object.oid.hash,\n \t\t\t\tDEFAULT_ABBREV));\n \t\tif (parse_commit(commit) != 0)\n-\t\t\tstrbuf_addf(&o->obuf, _(\"(bad commit)\\n\"));\n+\t\t\tstrbuf_addstr(&o->obuf, _(\"(bad commit)\\n\"));\n \t\telse {\n \t\t\tconst char *title;\n \t\t\tconst char *msg = get_commit_buffer(commit, NULL);\ndiff --git a/remote.c b/remote.c\nindex d29850a..ad6c542 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -2073,7 +2073,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n \t\t\t_(\"Your branch is based on '%s', but the upstream is gone.\\n\"),\n \t\t\tbase);\n \t\tif (advice_status_hints)\n-\t\t\tstrbuf_addf(sb,\n+\t\t\tstrbuf_addstr(sb,\n \t\t\t\t_(\"  (use \\\"git branch --unset-upstream\\\" to fixup)\\n\"));\n \t} else if (!ours && !theirs) {\n \t\tstrbuf_addf(sb,\n@@ -2086,7 +2086,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n \t\t\t   ours),\n \t\t\tbase, ours);\n \t\tif (advice_status_hints)\n-\t\t\tstrbuf_addf(sb,\n+\t\t\tstrbuf_addstr(sb,\n \t\t\t\t_(\"  (use \\\"git push\\\" to publish your local commits)\\n\"));\n \t} else if (!ours) {\n \t\tstrbuf_addf(sb,\n@@ -2097,7 +2097,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n \t\t\t   theirs),\n \t\t\tbase, theirs);\n \t\tif (advice_status_hints)\n-\t\t\tstrbuf_addf(sb,\n+\t\t\tstrbuf_addstr(sb,\n \t\t\t\t_(\"  (use \\\"git pull\\\" to update your local branch)\\n\"));\n \t} else {\n \t\tstrbuf_addf(sb,\n@@ -2110,7 +2110,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)\n \t\t\t   ours + theirs),\n \t\t\tbase, ours, theirs);\n \t\tif (advice_status_hints)\n-\t\t\tstrbuf_addf(sb,\n+\t\t\tstrbuf_addstr(sb,\n \t\t\t\t_(\"  (use \\\"git pull\\\" to merge the remote branch into yours)\\n\"));\n \t}\n \tfree(base);\ndiff --git a/wt-status.c b/wt-status.c\nindex 539aac1..f928f0f 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -367,11 +367,11 @@ static void wt_longstatus_print_change_data(struct wt_status *s,\n \t\tif (d->new_submodule_commits || d->dirty_submodule) {\n \t\t\tstrbuf_addstr(&extra, \" (\");\n \t\t\tif (d->new_submodule_commits)\n-\t\t\t\tstrbuf_addf(&extra, _(\"new commits, \"));\n+\t\t\t\tstrbuf_addstr(&extra, _(\"new commits, \"));\n \t\t\tif (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n-\t\t\t\tstrbuf_addf(&extra, _(\"modified content, \"));\n+\t\t\t\tstrbuf_addstr(&extra, _(\"modified content, \"));\n \t\t\tif (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n-\t\t\t\tstrbuf_addf(&extra, _(\"untracked content, \"));\n+\t\t\t\tstrbuf_addstr(&extra, _(\"untracked content, \"));\n \t\t\tstrbuf_setlen(&extra, extra.len - 2);\n \t\t\tstrbuf_addch(&extra, ')');\n \t\t}\n-- \n2.10.0\n\n"},{"id":"301995","messageId":"20160915184448.awipvg2kmlq7weei@sigill.intra.peff.net","threadId":"44094","inReplyTo":"f7294ac5-8302-03fb-d756-81a1c029a813@web.de","subject":"Re: [PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-15T18:44:48Z","receivedAt":"2016-09-15T18:44:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 15, 2016 at 08:31:00PM +0200, René Scharfe wrote:\n\n> Replace uses of strbuf_addf() for adding strings with more lightweight\n> strbuf_addstr() calls.  This makes the intent clearer and avoids\n> potential issues with printf format specifiers.\n> \n> 02962d36845b89145cd69f8bc65e015d78ae3434 already converted six cases,\n> this patch covers eleven more.\n\nGreat, these all look obviously correct.\n\n> A semantic patch for Coccinelle is included for easier checking for\n> new cases that might be introduced in the future.\n\nI think there was some discussion in brian's object_id patches about\nwhether we wanted to carry Coccinelle transformations in the tree, but I\ndon't remember the outcome. I don't have an opinion myself.\n\n> Silly question: Is there a natural language that uses percent signs\n> as letters or e.g. instead of commas? :)\n\nI don't know, but if they do, they'd better get used to escaping them.\n:)\n\n-Peff\n"},{"id":"302002","messageId":"xmqqbmzpnex4.fsf@gitster.mtv.corp.google.com","threadId":"44094","inReplyTo":"20160915184448.awipvg2kmlq7weei@sigill.intra.peff.net","subject":"Re: [PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-15T19:25:43Z","receivedAt":"2016-09-15T19:25:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, Sep 15, 2016 at 08:31:00PM +0200, René Scharfe wrote:\n>\n>> Replace uses of strbuf_addf() for adding strings with more lightweight\n>> strbuf_addstr() calls.  This makes the intent clearer and avoids\n>> potential issues with printf format specifiers.\n>> \n>> 02962d36845b89145cd69f8bc65e015d78ae3434 already converted six cases,\n>> this patch covers eleven more.\n>\n> Great, these all look obviously correct.\n\nYes.\n\n>> Silly question: Is there a natural language that uses percent signs\n>> as letters or e.g. instead of commas? :)\n>\n> I don't know, but if they do, they'd better get used to escaping them.\n> :)\n\nI do not know either, but I am curious where that question comes\nfrom.  I stared at this patch for a few minutes but couldn't guess.\n"},{"id":"302006","messageId":"20160915193804.d2mmmeard2rj6vye@sigill.intra.peff.net","threadId":"44094","inReplyTo":"xmqqbmzpnex4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-15T19:38:04Z","receivedAt":"2016-09-15T19:38:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 15, 2016 at 12:25:43PM -0700, Junio C Hamano wrote:\n\n> >> Silly question: Is there a natural language that uses percent signs\n> >> as letters or e.g. instead of commas? :)\n> >\n> > I don't know, but if they do, they'd better get used to escaping them.\n> > :)\n> \n> I do not know either, but I am curious where that question comes\n> from.  I stared at this patch for a few minutes but couldn't guess.\n\nMy initial thought is that the next step after picking this low-hanging\nfruit would be to find cases where the strings do not contain \"%\", and\nthus we do not have to care about formatting. But a case like:\n\n  strbuf_addf(&buf, \"this does not have any percents!\", foo);\n\nis simply broken (albeit in a way that we ignore foo, so it's just ugly\ncode, not a real bug).\n\nSo I dunno. I too am curious.\n\n-Peff\n"},{"id":"302008","messageId":"79f6cfb5-3b9b-2ae7-d9a3-5c1c65c7d4cf@web.de","threadId":"44094","inReplyTo":"20160915193804.d2mmmeard2rj6vye@sigill.intra.peff.net","subject":"Re: [PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-09-15T19:55:09Z","receivedAt":"2016-09-15T19:55:58Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 15.09.2016 um 21:38 schrieb Jeff King:\n> On Thu, Sep 15, 2016 at 12:25:43PM -0700, Junio C Hamano wrote:\n>\n>>>> Silly question: Is there a natural language that uses percent signs\n>>>> as letters or e.g. instead of commas? :)\n>>>\n>>> I don't know, but if they do, they'd better get used to escaping them.\n>>> :)\n>>\n>> I do not know either, but I am curious where that question comes\n>> from.  I stared at this patch for a few minutes but couldn't guess.\n>\n> My initial thought is that the next step after picking this low-hanging\n> fruit would be to find cases where the strings do not contain \"%\", and\n> thus we do not have to care about formatting. But a case like:\n>\n>   strbuf_addf(&buf, \"this does not have any percents!\", foo);\n>\n> is simply broken (albeit in a way that we ignore foo, so it's just ugly\n> code, not a real bug).\n>\n> So I dunno. I too am curious.\n\nTake this for example:\n\n-\t\t\tstrbuf_addf(&o->obuf, _(\"(bad commit)\\n\"));\n+\t\t\tstrbuf_addstr(&o->obuf, _(\"(bad commit)\\n\"));\n\nIf there's a language that uses percent signs instead of parens or as \nregular letters, then they need to be escaped in the translated string \nbefore, but not after the patch.  As I wrote: silly.\n\nRené\n"},{"id":"302009","messageId":"xmqq7fadnd9s.fsf@gitster.mtv.corp.google.com","threadId":"44094","inReplyTo":"79f6cfb5-3b9b-2ae7-d9a3-5c1c65c7d4cf@web.de","subject":"Re: [PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-15T20:01:19Z","receivedAt":"2016-09-15T20:01:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Take this for example:\n>\n> -\t\t\tstrbuf_addf(&o->obuf, _(\"(bad commit)\\n\"));\n> +\t\t\tstrbuf_addstr(&o->obuf, _(\"(bad commit)\\n\"));\n>\n> If there's a language that uses percent signs instead of parens or as\n> regular letters, then they need to be escaped in the translated string\n> before, but not after the patch.  As I wrote: silly.\n\nAhh, OK, so \"This use of addf only has format part and nothing else,\nhence the format part can be taken as-is\" which is the Coccinelle rule\nused to produce this patch is incomplete and always needs manual\ninspection, in case the format part wanted to give a literal % in\nthe output.  E.g. it is a bug to convert this\n\n\tstrbuf_addf(&buf, _(\"this is 100%% wrong!\"));\n\nto\n\n\tstrbuf_addstr(&buf, _(\"this is 100%% wrong!\"));\n\nThanks for clarification.  Perhaps the strbuf.cocci rule file can\nhave some comment to warn the person who builds *.patch file to look\nfor % in E2, or something?\n\n"},{"id":"302027","messageId":"67756074-836f-2238-37c3-0d186325bd00@web.de","threadId":"44094","inReplyTo":"xmqq7fadnd9s.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-09-15T21:25:55Z","receivedAt":"2016-09-15T21:26:40Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 15.09.2016 um 22:01 schrieb Junio C Hamano:\n> René Scharfe <l.s.r@web.de> writes:\n> \n>> Take this for example:\n>>\n>> -\t\t\tstrbuf_addf(&o->obuf, _(\"(bad commit)\\n\"));\n>> +\t\t\tstrbuf_addstr(&o->obuf, _(\"(bad commit)\\n\"));\n>>\n>> If there's a language that uses percent signs instead of parens or as\n>> regular letters, then they need to be escaped in the translated string\n>> before, but not after the patch.  As I wrote: silly.\n> \n> Ahh, OK, so \"This use of addf only has format part and nothing else,\n> hence the format part can be taken as-is\" which is the Coccinelle rule\n> used to produce this patch is incomplete and always needs manual\n> inspection, in case the format part wanted to give a literal % in\n> the output.  E.g. it is a bug to convert this\n> \n> \tstrbuf_addf(&buf, _(\"this is 100%% wrong!\"));\n> \n> to\n> \n> \tstrbuf_addstr(&buf, _(\"this is 100%% wrong!\"));\n\nRight.  Such strings seem to be quite rare in practice, though. \n\n> Thanks for clarification.  Perhaps the strbuf.cocci rule file can\n> have some comment to warn the person who builds *.patch file to look\n> for % in E2, or something?\n\nSomething like this?\n\n---\n contrib/coccinelle/strbuf.cocci | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci\nindex 7932d48..3f535ca 100644\n--- a/contrib/coccinelle/strbuf.cocci\n+++ b/contrib/coccinelle/strbuf.cocci\n@@ -1,3 +1,5 @@\n+// Careful, this is not fully equivalent: \"%\" is no longer treated\n+// specially.  Check for \"%%\", \"%m\" etc. in the format string (E2).\n @@\n expression E1, E2;\n @@\n-- \n2.10.0\n\n\n\n"},{"id":"302029","messageId":"xmqqwpiclu65.fsf@gitster.mtv.corp.google.com","threadId":"44094","inReplyTo":"67756074-836f-2238-37c3-0d186325bd00@web.de","subject":"Re: [PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-09-15T21:39:14Z","receivedAt":"2016-09-15T21:39:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Am 15.09.2016 um 22:01 schrieb Junio C Hamano:\n>> René Scharfe <l.s.r@web.de> writes:\n>> \n>>> Take this for example:\n>>>\n>>> -\t\t\tstrbuf_addf(&o->obuf, _(\"(bad commit)\\n\"));\n>>> +\t\t\tstrbuf_addstr(&o->obuf, _(\"(bad commit)\\n\"));\n>>>\n>>> If there's a language that uses percent signs instead of parens or as\n>>> regular letters, then they need to be escaped in the translated string\n>>> before, but not after the patch.  As I wrote: silly.\n>> \n>> Ahh, OK, so \"This use of addf only has format part and nothing else,\n>> hence the format part can be taken as-is\" which is the Coccinelle rule\n>> used to produce this patch is incomplete and always needs manual\n>> inspection, in case the format part wanted to give a literal % in\n>> the output.  E.g. it is a bug to convert this\n>> \n>> \tstrbuf_addf(&buf, _(\"this is 100%% wrong!\"));\n>> \n>> to\n>> \n>> \tstrbuf_addstr(&buf, _(\"this is 100%% wrong!\"));\n>\n> Right.  Such strings seem to be quite rare in practice, though. \n>\n>> Thanks for clarification.  Perhaps the strbuf.cocci rule file can\n>> have some comment to warn the person who builds *.patch file to look\n>> for % in E2, or something?\n>\n> Something like this?\n\nYup, with something like that I would understdood where that\npuzzling question came from.\n\nThanks.\n\n>\n> ---\n>  contrib/coccinelle/strbuf.cocci | 2 ++\n>  1 file changed, 2 insertions(+)\n>\n> diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci\n> index 7932d48..3f535ca 100644\n> --- a/contrib/coccinelle/strbuf.cocci\n> +++ b/contrib/coccinelle/strbuf.cocci\n> @@ -1,3 +1,5 @@\n> +// Careful, this is not fully equivalent: \"%\" is no longer treated\n> +// specially.  Check for \"%%\", \"%m\" etc. in the format string (E2).\n>  @@\n>  expression E1, E2;\n>  @@\n"},{"id":"302032","messageId":"20160915234731.3oqd6x7zxtcxuk3h@vauxhall.crustytoothpaste.net","threadId":"44094","inReplyTo":"f7294ac5-8302-03fb-d756-81a1c029a813@web.de","subject":"Re: [PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2016-09-15T23:47:31Z","receivedAt":"2016-09-15T23:47:42Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Thu, Sep 15, 2016 at 08:31:00PM +0200, René Scharfe wrote:\n> Replace uses of strbuf_addf() for adding strings with more lightweight\n> strbuf_addstr() calls.  This makes the intent clearer and avoids\n> potential issues with printf format specifiers.\n> \n> 02962d36845b89145cd69f8bc65e015d78ae3434 already converted six cases,\n> this patch covers eleven more.\n> \n> A semantic patch for Coccinelle is included for easier checking for\n> new cases that might be introduced in the future.\n\nI think all three of these patches look good.  I'm glad to see us\ngetting better use out of Coccinelle.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"303070","messageId":"9014c48e-9b9d-995b-47ba-9ecc48a10ac7@web.de","threadId":"44094","inReplyTo":"xmqqwpiclu65.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] use strbuf_addstr() for adding constant strings to a strbuf, part 2","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2016-10-02T22:58:21Z","receivedAt":"2016-10-02T22:59:05Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 15.09.2016 um 23:39 schrieb Junio C Hamano:\n> René Scharfe <l.s.r@web.de> writes:\n> \n>> Am 15.09.2016 um 22:01 schrieb Junio C Hamano:\n>>> René Scharfe <l.s.r@web.de> writes:\n>>>\n>>>> Take this for example:\n>>>>\n>>>> -\t\t\tstrbuf_addf(&o->obuf, _(\"(bad commit)\\n\"));\n>>>> +\t\t\tstrbuf_addstr(&o->obuf, _(\"(bad commit)\\n\"));\n>>>>\n>>>> If there's a language that uses percent signs instead of parens or as\n>>>> regular letters, then they need to be escaped in the translated string\n>>>> before, but not after the patch.  As I wrote: silly.\n>>>\n>>> Ahh, OK, so \"This use of addf only has format part and nothing else,\n>>> hence the format part can be taken as-is\" which is the Coccinelle rule\n>>> used to produce this patch is incomplete and always needs manual\n>>> inspection, in case the format part wanted to give a literal % in\n>>> the output.  E.g. it is a bug to convert this\n>>>\n>>> \tstrbuf_addf(&buf, _(\"this is 100%% wrong!\"));\n>>>\n>>> to\n>>>\n>>> \tstrbuf_addstr(&buf, _(\"this is 100%% wrong!\"));\n>>\n>> Right.  Such strings seem to be quite rare in practice, though. \n>>\n>>> Thanks for clarification.  Perhaps the strbuf.cocci rule file can\n>>> have some comment to warn the person who builds *.patch file to look\n>>> for % in E2, or something?\n>>\n>> Something like this?\n> \n> Yup, with something like that I would understdood where that\n> puzzling question came from.\n\nHere's something better than a comment:\n\n-- >8 --\nSubject: [PATCH] coccicheck: make transformation for strbuf_addf(sb, \"...\") more precise\n\nWe can replace strbuf_addf() calls that just add a simple string with\ncalls to strbuf_addstr() to make the intent clearer.  We need to be\ncareful if that string contains printf format specifications like %%,\nthough, as a simple replacement would change the output.\n\nAdd checks to the semantic patch to make sure we only perform the\ntransformation if the second argument is a string constant (possibly\ntranslated) that doesn't contain any percent signs.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n contrib/coccinelle/strbuf.cocci | 29 ++++++++++++++++++++++++++---\n 1 file changed, 26 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci\nindex 1e24298..63995f2 100644\n--- a/contrib/coccinelle/strbuf.cocci\n+++ b/contrib/coccinelle/strbuf.cocci\n@@ -1,8 +1,31 @@\n+@ strbuf_addf_with_format_only @\n+expression E;\n+constant fmt;\n @@\n-expression E1, E2;\n+  strbuf_addf(E,\n+(\n+  fmt\n+|\n+  _(fmt)\n+)\n+  );\n+\n+@ script:python @\n+fmt << strbuf_addf_with_format_only.fmt;\n @@\n-- strbuf_addf(E1, E2);\n-+ strbuf_addstr(E1, E2);\n+cocci.include_match(\"%\" not in fmt)\n+\n+@ extends strbuf_addf_with_format_only @\n+@@\n+- strbuf_addf\n++ strbuf_addstr\n+  (E,\n+(\n+  fmt\n+|\n+  _(fmt)\n+)\n+  );\n \n @@\n expression E1, E2;\n-- \n2.10.0\n"}]}