[PATCH] ipw2x00: Remove a memory allocation failure log message

[PATCH] ipw2x00: Remove a memory allocation failure log message

[Christophe JAILLET]

Axe a memory allocation failure log message. This message is useless and
incorrect (vmalloc is not used here for the memory allocation)

This has been like that since the very beginning of this driver in
commit 43f66a6ce8da ("Add ipw2200 wireless driver.")

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/net/wireless/intel/ipw2x00/ipw2200.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
index 60b5e08dd6df..30c4f041f565 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
@@ -3770,10 +3770,9 @@ static int ipw_queue_tx_init(struct ipw_priv *priv,
 	struct pci_dev *dev = priv->pci_dev;
 
 	q->txb = kmalloc_array(count, sizeof(q->txb[0]), GFP_KERNEL);
-	if (!q->txb) {
-		IPW_ERROR("vmalloc for auxiliary BD structures failed\n");
+	if (!q->txb)
 		return -ENOMEM;
-	}
+
 
 	q->bd =
 	    pci_alloc_consistent(dev, sizeof(q->bd[0]) * count, &q->q.dma_addr);
-- 
2.20.1

Re: [PATCH] ipw2x00: Remove a memory allocation failure log message

[Joe Perches]

On Thu, 2020-04-23 at 09:58 +0200, Christophe JAILLET wrote:
> Axe a memory allocation failure log message. This message is useless and
> incorrect (vmalloc is not used here for the memory allocation)
> 
> This has been like that since the very beginning of this driver in
> commit 43f66a6ce8da ("Add ipw2200 wireless driver.")
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>  drivers/net/wireless/intel/ipw2x00/ipw2200.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
> index 60b5e08dd6df..30c4f041f565 100644
> --- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
> +++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
> @@ -3770,10 +3770,9 @@ static int ipw_queue_tx_init(struct ipw_priv *priv,
>  	struct pci_dev *dev = priv->pci_dev;
>  
>  	q->txb = kmalloc_array(count, sizeof(q->txb[0]), GFP_KERNEL);
> -	if (!q->txb) {
> -		IPW_ERROR("vmalloc for auxiliary BD structures failed\n");
> +	if (!q->txb)
>  		return -ENOMEM;
> -	}
> +

This might avoid possible defects by using kcalloc instead too

Re: [PATCH] ipw2x00: Remove a memory allocation failure log message

[Sergei Shtylyov]

Hello!

On 23.04.2020 10:58, Christophe JAILLET wrote:

> Axe a memory allocation failure log message. This message is useless and
> incorrect (vmalloc is not used here for the memory allocation)
> 
> This has been like that since the very beginning of this driver in
> commit 43f66a6ce8da ("Add ipw2200 wireless driver.")
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>   drivers/net/wireless/intel/ipw2x00/ipw2200.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
> index 60b5e08dd6df..30c4f041f565 100644
> --- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
> +++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
> @@ -3770,10 +3770,9 @@ static int ipw_queue_tx_init(struct ipw_priv *priv,
>   	struct pci_dev *dev = priv->pci_dev;
>   
>   	q->txb = kmalloc_array(count, sizeof(q->txb[0]), GFP_KERNEL);
> -	if (!q->txb) {
> -		IPW_ERROR("vmalloc for auxiliary BD structures failed\n");
> +	if (!q->txb)
>   		return -ENOMEM;
> -	}
> +

    No need for this extra empty line.

>   
>   	q->bd =
>   	    pci_alloc_consistent(dev, sizeof(q->bd[0]) * count, &q->q.dma_addr);

MBR, Sergei

Re: [PATCH] ipw2x00: Remove a memory allocation failure log message

[Christophe JAILLET]

Le 23/04/2020 à 11:46, Sergei Shtylyov a écrit :
> Hello!
>
> On 23.04.2020 10:58, Christophe JAILLET wrote:
>
>> Axe a memory allocation failure log message. This message is useless and
>> incorrect (vmalloc is not used here for the memory allocation)
>>
>> This has been like that since the very beginning of this driver in
>> commit 43f66a6ce8da ("Add ipw2200 wireless driver.")
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>>   drivers/net/wireless/intel/ipw2x00/ipw2200.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c 
>> b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>> index 60b5e08dd6df..30c4f041f565 100644
>> --- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>> +++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>> @@ -3770,10 +3770,9 @@ static int ipw_queue_tx_init(struct ipw_priv 
>> *priv,
>>       struct pci_dev *dev = priv->pci_dev;
>>         q->txb = kmalloc_array(count, sizeof(q->txb[0]), GFP_KERNEL);
>> -    if (!q->txb) {
>> -        IPW_ERROR("vmalloc for auxiliary BD structures failed\n");
>> +    if (!q->txb)
>>           return -ENOMEM;
>> -    }
>> +
>
>    No need for this extra empty line.


That's right, sorry about that.

Can it be fixed when/if the patch is applied, or should I send a V2?
If a V2 is required, should kcalloc be used, as pointed out by Joe Perches?
(personally, If the code works fine as-is, I don't think it is required, 
but it can't hurt)

CJ

>
>>         q->bd =
>>           pci_alloc_consistent(dev, sizeof(q->bd[0]) * count, 
>> &q->q.dma_addr);
>
> MBR, Sergei
>
>

Re: [PATCH] ipw2x00: Remove a memory allocation failure log message

[Kalle Valo]

Christophe JAILLET <christophe.jaillet@wanadoo.fr> writes:

> Le 23/04/2020 à 11:46, Sergei Shtylyov a écrit :
>> Hello!
>>
>> On 23.04.2020 10:58, Christophe JAILLET wrote:
>>
>>> Axe a memory allocation failure log message. This message is useless and
>>> incorrect (vmalloc is not used here for the memory allocation)
>>>
>>> This has been like that since the very beginning of this driver in
>>> commit 43f66a6ce8da ("Add ipw2200 wireless driver.")
>>>
>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>>> ---
>>>   drivers/net/wireless/intel/ipw2x00/ipw2200.c | 5 ++---
>>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> index 60b5e08dd6df..30c4f041f565 100644
>>> --- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> +++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
>>> @@ -3770,10 +3770,9 @@ static int ipw_queue_tx_init(struct ipw_priv
>>> *priv,
>>>       struct pci_dev *dev = priv->pci_dev;
>>>         q->txb = kmalloc_array(count, sizeof(q->txb[0]), GFP_KERNEL);
>>> -    if (!q->txb) {
>>> -        IPW_ERROR("vmalloc for auxiliary BD structures failed\n");
>>> +    if (!q->txb)
>>>           return -ENOMEM;
>>> -    }
>>> +
>>
>>    No need for this extra empty line.
>
>
> That's right, sorry about that.
>
> Can it be fixed when/if the patch is applied, or should I send a V2?

Please send v2.

> If a V2 is required, should kcalloc be used, as pointed out by Joe Perches?
> (personally, If the code works fine as-is, I don't think it is
> required, but it can't hurt)

There's always the risk of regressions, which happens even with cleanup
patches so hurting is always possible :)

I can take a patch changing the allocation but please do it in a
separate patch. Though personally I wouldn't bother, ipw2x00 is an old
driver and not being actively developed anymore.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches