Hi Teng, On Wed, 5 Jan 2022, Teng Long wrote: > Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes: > > > This, along with two other similar instances, triggers the > > `static-analysis` job in the CI failure of `seen`. The suggested diff is: > > The second and third I will optimize in the next patch. > > The first one. Actually I am a little puzzled from this : > > > - strbuf_addf(line, "%7s", "-"); > > + strbuf_addstr(line, "-"); > > > But I think that the first hunk indicates a deeper issue, as `%7s` > > probably meant to pad the dash to seven dashes (which that format won't > > accomplish, but `strbuf_addchars()` would)? > > "strbuf_addf(line, "%7s", "-");" here is used to align the columns > with a width of seven chars, not repeat one DASH to seven. Ah. I misremembered and thought that `"% 7s"` would do that, but you're correct. See below for more on this. But first, I wonder why the test suite passes with the `strbuf_addstr()` call... Is this line not covered by any test case? About the `%7s` thing: The most obvious resolution is to use `" -"` with `strbuf_addstr()`. And I would argue that this is the best resolution. If you disagree (and want to spin up a full `sprintf()` every time, just to add those six space characters), feel free to integrate the following into your patch series: -- snip -- >From a390fcf7eec261c7f0e341bda79f2b1f326d151e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin <johannes.schindelin@xxxxxx> Date: Wed, 5 Jan 2022 14:02:19 +0100 Subject: [PATCH] cocci: allow padding with `strbuf_addf()` A convenient way to pad strings is to use something like `strbuf_addf(&buf, "%20s", "Hello, world!")`. However, the Coccinelle rule that forbids a format `"%s"` with a constant string argument cast too wide a net, and also forbade such padding. Let's be a bit stricter in that Coccinelle rule. Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx> --- contrib/coccinelle/strbuf.cocci | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci index d9ada69b432..2d6e0f58fc8 100644 --- a/contrib/coccinelle/strbuf.cocci +++ b/contrib/coccinelle/strbuf.cocci @@ -44,7 +44,7 @@ struct strbuf *SBP; @@ expression E1, E2; -format F =~ "s"; +format F =~ "^s$"; @@ - strbuf_addf(E1, "%@F@", E2); + strbuf_addstr(E1, E2); -- 2.33.0.windows.2 -- snap -- Ciao, Dscho > > A little weird about the fix recommendation of "strbuf_addstr(line, "-");" , > because it will only add a single DASH here. > > It's the identical result which compares to the "master"[1] I think with the > current codes and I tested the "strbuf_addf()" simply and it seems to work > fine. > > [1] https://github.com/git/git/blob/master/builtin/ls-tree.c#L106
