Re: [PATCH] describe: use strbuf_add_unique_abbrev() for adding short hashes

From: René Scharfe <l.s.r@web.de>
Date: Jan 19, 2018, 17:53 UTC
Message-ID: <d81743e5-d5ba-a565-23f7-072007493499@web.de>
In-Reply-To: <CAM0VKj=eL5ZnnwKrxqtDYE+ijqJvcU-4bJNhjurvdn+Bpv7gUQ@mail.gmail.com>

Am 18.01.2018 um 23:40 schrieb SZEDER Gábor:

Show 22 quoted lines

> On Thu, Jan 18, 2018 at 10:40 PM, René Scharfe <l.s.r@web.de> wrote:
>> Am 16.01.2018 um 18:11 schrieb SZEDER Gábor:
>>> Unfortunately, most of the changes coming from 'strbuf.cocci' don't
>>> make any sense, they appear to be the mis-application of the "use
>>> strbuf_addstr() instead of strbuf_addf() to add a single string" rule:
>>>
>>>     -             strbuf_addf(&sb_repo, "%d", counter);
>>>     +             strbuf_addstr(&sb_repo, counter);
>>>
>>> It seems that those rules need some refinement, but I have no idea
>>> about Coccinelle and this is not the time for me to dig deeper.
>>>
>>> What makes all this weird is that running 'make coccicheck' on my own
>>> machine doesn't produce any of these additional proposed changes, just
>>> like at René's.  Can it be related to differing Coccinelle versions?
>>> Travis CI installs 1.0.0~rc19.deb-3; I have 1.0.4.deb-2.
>>
>> The version difference may explain it, but I couldn't find a matching
>> bugfix in http://coccinelle.lip6.fr/distrib/changes.html when I just
>> skimmed it.  I wonder if the following patch could make a difference:
> 
> Yes, it does, now all those nonsense suggestions are gone on Travis CI.

I would have expected matching a literal "%s" to be easier than dissecting that (admittedly simple) format string, but if it all works out fine then I'm not complaining. :) Sent the patch again properly.

>    https://travis-ci.org/szeder/git/jobs/330572425#L713
> 
> Those "memmove() -> MOVE_ARRAY" suggestions are still there, of course.

They look valid and nice to have in that report. I wonder why we don't get them locally, though. Are you going to submit them as a patch?

(NB: The patches generated by coccicheck apply with "patch -p0", unlike
 those generated by git diff and friends.)

Thanks, René