Re: [PATCH] promisor-remote: fix xcalloc() argument order
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Aug 23, 2022, 09:56 UTC
- Message-ID
- <20220823095602.GC1735@szeder.dev>
- In-Reply-To
- <20220823092156.GB1735@szeder.dev>
On Tue, Aug 23, 2022 at 11:21:56AM +0200, SZEDER Gábor wrote:
Show 10 quoted lines
> On Tue, Aug 23, 2022 at 09:04:17AM +0200, SZEDER Gábor wrote: > > 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,
Show 45 quoted lines
> # 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.
Ok, I think I got this.
The transformation involves 'r->promisor_remote_config', where 'r' is a 'struct repository*'. However, 'promisor-remote.c' doesn't include 'repository.h' directly:
$ grep '^#include' promisor-remote.c #include "cache.h" #include "object-store.h" #include "promisor-remote.h" #include "config.h" #include "transport.h" #include "strvec.h"
If I add an '#include "repository.h" to 'promisor-remote.c', then Coccinelle finds the transformation even when it processes this source file on is own:
$ spatch --all-includes --sp-file contrib/coccinelle/xcalloc.cocci --patch . promisor-remote.c init_defs_builtins: /usr/local/lib/coccinelle/standard.h HANDLING: promisor-remote.c diff = diff -u -p a/promisor-remote.c b/promisor-remote.c --- a/promisor-remote.c +++ b/promisor-remote.c @@ -149,7 +149,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);