netdev - Re: [PATCH net-next 04/13] sunhme: Return an ERR_PTR from quattro_pci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0ffba41b-3808-c2d8-e180-d865c8d5d306@gmail.com>
Date:   Mon, 19 Sep 2022 10:08:55 -0400
From:   Sean Anderson <seanga2@...il.com>
To:     Rolf Eike Beer <eike-kernel@...tec.de>,
        "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, Zheyu Ma <zheyuma97@...il.com>,
        Nick Bowler <nbowler@...conx.ca>
Subject: Re: [PATCH net-next 04/13] sunhme: Return an ERR_PTR from
 quattro_pci_find

On 9/19/22 09:11, Rolf Eike Beer wrote:
> Am Montag, 19. September 2022, 01:26:17 CEST schrieb Sean Anderson:
>> In order to differentiate between a missing bridge and an OOM condition,
>> return ERR_PTRs from quattro_pci_find. This also does some general linting
>> in the area.
>>
>> Signed-off-by: Sean Anderson <seanga2@...il.com>
>> ---
>>
>>   drivers/net/ethernet/sun/sunhme.c | 33 +++++++++++++++++++------------
>>   1 file changed, 20 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/sun/sunhme.c
>> b/drivers/net/ethernet/sun/sunhme.c index 1fc16801f520..52247505d08e 100644
>> --- a/drivers/net/ethernet/sun/sunhme.c
>> +++ b/drivers/net/ethernet/sun/sunhme.c
>> @@ -2569,30 +2569,33 @@ static void quattro_sbus_free_irqs(void)
>>   #ifdef CONFIG_PCI
>>   static struct quattro *quattro_pci_find(struct pci_dev *pdev)
>>   {
>> +	int i;
>>   	struct pci_dev *bdev = pdev->bus->self;
>>   	struct quattro *qp;
>>
>> -	if (!bdev) return NULL;
>> +	if (!bdev)
>> +		return ERR_PTR(-ENODEV);
>> +
>>   	for (qp = qfe_pci_list; qp != NULL; qp = qp->next) {
>>   		struct pci_dev *qpdev = qp->quattro_dev;
>>
>>   		if (qpdev == bdev)
>>   			return qp;
>>   	}
>> +
>>   	qp = kmalloc(sizeof(struct quattro), GFP_KERNEL);
>> -	if (qp != NULL) {
>> -		int i;
>> +	if (!qp)
>> +		return ERR_PTR(-ENOMEM);
>>
>> -		for (i = 0; i < 4; i++)
>> -			qp->happy_meals[i] = NULL;
>> +	for (i = 0; i < 4; i++)
>> +		qp->happy_meals[i] = NULL;
> 
> I know you are only reindenting it, but I dislike moving the variable up to
> the top of the function. Since the kernel is C99 meanwhile the variable could
> be declared just in the for loop. 

Hm, I thought this style was discouraged.

> And when touching this anyway I think we
> could get rid of the magic "4" by using ARRAY_SIZE(qp->happy_meals). Or just
> replace the whole thing with memset(qp->happy_meals, 0, sizeof(qp-
>> happy_meals)).

Yeah, that avoids the whole problem.

--Sean