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

[SZEDER Gábor <szeder.dev@xxxxxxxxx>]

On Tue, Aug 23, 2022 at 11:21:56AM +0200, SZEDER Gábor wrote:
> 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@xxxxxxxxx> 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,


> # 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);