Re: [PATCH] promisor-remote: fix xcalloc() argument order

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Aug 23, 2022, 09:21 UTC
Message-ID: <20220823092156.GB1735@szeder.dev>
In-Reply-To: <20220823070417.GA1735@szeder.dev>

On Tue, Aug 23, 2022 at 09:04:17AM +0200, SZEDER Gábor wrote:

Show 9 quoted lines

> On Mon, Aug 22, 2022 at 06:09:41PM -0700, Junio C Hamano wrote:
> > Junio C Hamano <gitster@pobox.com> writes:
> > 
> > > ...  FWIW, "make coccicheck" with what I happen to have
> > > notices it.
> > 
> > Oops, that was a serious typo.  "notices" -> "fails to notice".
> 
> Hm, that's strange.  1.1.1 did notice this transformation for me,

Hang on, it's not quite that simple. Watch this, it will get weird:

# Batched spatch for faster processing, see 960154b9c1 (coccicheck:
# optionally batch spatch invocations, 2019-05-06)
$ grep SPATCH_BATCH_SIZE config.mak
SPATCH_BATCH_SIZE = 32
$ time make contrib/coccinelle/xcalloc.cocci.patch    
    SPATCH contrib/coccinelle/xcalloc.cocci
     SPATCH result: contrib/coccinelle/xcalloc.cocci.patch

real	0m35.902s
user	0m34.840s
sys	0m1.030s
# Found it, good.
$ make -s cocciclean
# Turn off batched processing (this is the default)
$ sed -i -e '/SPATCH_BATCH_SIZE/ s/= .*/= 1/' config.mak
$ time make contrib/coccinelle/xcalloc.cocci.patch    
    SPATCH contrib/coccinelle/xcalloc.cocci

real 0m24.553s user 0m21.468s sys 0m3.099s # Not only missed the transformation, but it's faster than batched # processing?!

# Let's invoke spatch directly.
$ spatch --all-includes --sp-file contrib/coccinelle/xcalloc.cocci --patch . promisor-remote.c                               
warning: Can't find macro file: /usr/local/bin/lib/coccinelle/standard.h
warning: Can't find default iso file: /usr/local/bin/lib/coccinelle/standard.iso
HANDLING: promisor-remote.c
# Nope.
$ spatch --all-includes --sp-file contrib/coccinelle/xcalloc.cocci --patch . promisor-remote.c lockfile.c
warning: Can't find macro file: /usr/local/bin/lib/coccinelle/standard.h
warning: Can't find default iso file: /usr/local/bin/lib/coccinelle/standard.iso
HANDLING: promisor-remote.c lockfile.c
$ spatch --all-includes --sp-file contrib/coccinelle/xcalloc.cocci --patch . promisor-remote.c git.c     
warning: Can't find macro file: /usr/local/bin/lib/coccinelle/standard.h
warning: Can't find default iso file: /usr/local/bin/lib/coccinelle/standard.iso
HANDLING: promisor-remote.c git.c
$ spatch --all-includes --sp-file contrib/coccinelle/xcalloc.cocci --patch . promisor-remote.c usage.c
warning: Can't find macro file: /usr/local/bin/lib/coccinelle/standard.h
warning: Can't find default iso file: /usr/local/bin/lib/coccinelle/standard.iso
HANDLING: promisor-remote.c usage.c
# Nope, nope, nope.
$ spatch --all-includes --sp-file contrib/coccinelle/xcalloc.cocci --patch . promisor-remote.c builtin/*.c
[...]
# Nope!

# But watch this!
$ spatch --all-includes --sp-file contrib/coccinelle/xcalloc.cocci --patch . promisor-remote.c config.c
warning: Can't find macro file: /usr/local/bin/lib/coccinelle/standard.h
warning: Can't find default iso file: /usr/local/bin/lib/coccinelle/standard.iso
HANDLING: promisor-remote.c config.c
diff = 
diff -u -p a/promisor-remote.c b/promisor-remote.c

--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -146,7 +146,7 @@ static void promisor_remote_init(struct
 	if (r->promisor_remote_config)
 		return;
 	config = r->promisor_remote_config =
-		xcalloc(sizeof(*r->promisor_remote_config), 1);
+		xcalloc(1, sizeof(*r->promisor_remote_config));
 	config->promisors_tail = &config->promisors;
 
 	repo_config(r, promisor_remote_config, config);

# Huh?! (;

FWIW, I see this with Coccinelle 1.1.1, 1.0.8 and 1.0.6 as well.