Subject: Re: [PATCH v3 1/3] gianfar: Less function calls in gfar_ethflow_to_filer_table() after error detection

From: Joe Perches <[email-masked]>
Date: Fri, 15 Jan 2016 02:37:09 -0800

On Fri, 2016-01-15 at 11:11 +0100, SF Markus Elfring wrote:
> The kfree() function was called in one case by the
> gfar_ethflow_to_filer_table() function during error handling
> even if a passed variable contained a null pointer.
> 
> * Return directly if a memory allocation failed at the beginning.
> 
> * Adjust jump targets according to the Linux coding style convention.
> 
> This issue was detected by using the Coccinelle software.

Is this really better?

Perhaps this particular static analysis isn't too useful.

Why not just allocate once and assign a second pointer?

	local_rqfpr = kmalloc_array(2 * (MAX_FILER_IDX + 1),
				    sizeof(unsigned int), GFP_KERNEL);
	if (!local_rqfpr)
		goto err;

	local_rqfcr = &local_rqfpr[MAX_FILER_IDX + 1];

Perhaps this would be better removing the ret variable
and using something like:

int gfar_ethflow_to_filer_table(...)
{
	...

	return 0;

err:
	kfree(local_rqfpt);
	return 1;
}