RE: [PATCH] Drivers: hv: vmbus: One function call less in create_gpadl_header() after error detection

[Michael Kelley]

From: Markus Elfring <Markus.Elfring@xxxxxx> Sent: Tuesday, December 26, 2023 11:09 AM
> 
> The kfree() function was called in two cases by
> the create_gpadl_header() function during error handling
> even if the passed variable contained a null pointer.
> This issue was detected by using the Coccinelle software.
> 
> Thus use another label.

Interestingly, there's a third case in this function where
"goto nomem" is done, and in this case, msgbody is NULL.
Does Coccinelle not complain about that case as well?

As I'm sure you know, the code is correct as is, because kfree()
checks for a NULL argument.  So this is really an exercise in
making Coccinelle happy.  To me, the additional label is
incremental complexity for someone to deal with when
reading the code at some time in the future.  So I'd vote for
leaving the code as is.  But it's not a big deal either way.  I
can see you've been cleaning up a lot of Coccinelle-reported
issues across the kernel, most of which result in code
simplifications.  If leaving this unchanged causes you problems,
then I won't object (though perhaps that 3rd "goto nomem"
should be dealt with as well for consistency).

Michael

> 
> Signed-off-by: Markus Elfring <elfring@xxxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/hv/channel.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 56f7e06c673e..4d1bbda895d8 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -336,7 +336,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>  			  sizeof(struct gpa_range) + pfncount * sizeof(u64);
>  		msgheader =  kzalloc(msgsize, GFP_KERNEL);
>  		if (!msgheader)
> -			goto nomem;
> +			goto free_body;
> 
>  		INIT_LIST_HEAD(&msgheader->submsglist);
>  		msgheader->msgsize = msgsize;
> @@ -417,7 +417,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>  			  sizeof(struct gpa_range) + pagecount * sizeof(u64);
>  		msgheader = kzalloc(msgsize, GFP_KERNEL);
>  		if (msgheader == NULL)
> -			goto nomem;
> +			goto free_body;
> 
>  		INIT_LIST_HEAD(&msgheader->submsglist);
>  		msgheader->msgsize = msgsize;
> @@ -439,6 +439,7 @@ static int create_gpadl_header(enum hv_gpadl_type type, void *kbuffer,
>  	return 0;
>  nomem:
>  	kfree(msgheader);
> +free_body:
>  	kfree(msgbody);
>  	return -ENOMEM;
>  }
> --
> 2.43.0
>