netdev - Re: [PATCH 4/x] sunhme: switch to devres

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: <812378b7-80ed-e966-248c-d00a3377a333@gmail.com>
Date:   Wed, 24 Aug 2022 11:57:22 -0400
From:   Sean Anderson <seanga2@...il.com>
To:     Rolf Eike Beer <eike-kernel@...tec.de>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH 4/x] sunhme: switch to devres

On 8/24/22 11:45 AM, Rolf Eike Beer wrote:
> Am Montag, 1. August 2022, 17:14:39 CEST schrieb Rolf Eike Beer:
>> Am Freitag, 29. Juli 2022, 02:33:01 CEST schrieb Sean Anderson:
>>> On 7/28/22 3:52 PM, Rolf Eike Beer wrote:
>>>> Am 2022-07-27 05:58, schrieb Sean Anderson:
>>>>> On 7/26/22 11:49 PM, Sean Anderson wrote:
>>>>>> This looks good, but doesn't apply cleanly. I rebased it as follows:
>>>> Looks like what my local rebase has also produced.
>>>>
>>>> The sentence about the leak from the commitmessage can be dropped then,
>>>> as this leak has already been fixed.
>>>>
>>>>>> diff --git a/drivers/net/ethernet/sun/sunhme.c
>>>>>> b/drivers/net/ethernet/sun/sunhme.c index eebe8c5f480c..e83774ffaa7a
>>>>>> 100644
>>>>>> --- a/drivers/net/ethernet/sun/sunhme.c
>>>>>> +++ b/drivers/net/ethernet/sun/sunhme.c
>>>>>> @@ -2990,21 +2990,23 @@ static int happy_meal_pci_probe(struct pci_dev
>>>>>> *pdev, qp->happy_meals[qfe_slot] = dev;
>>>>>>
>>>>>>        }
>>>>>>
>>>>>> -    hpreg_res = pci_resource_start(pdev, 0);
>>>>>> -    err = -ENODEV;
>>>>>>
>>>>>>        if ((pci_resource_flags(pdev, 0) & IORESOURCE_IO) != 0) {
>>>>>>        
>>>>>>            printk(KERN_ERR "happymeal(PCI): Cannot find proper PCI
>>>>>>            device
>>>>>>
>>>>>> base address.\n"); goto err_out_clear_quattro;
>>>>>>
>>>>>>        }
>>>>>>
>>>>>> -    if (pci_request_regions(pdev, DRV_NAME)) {
>>>>>> +
>>>>>> +    if (!devm_request_region(&pdev->dev, pci_resource_start(pdev, 0),
>>>>>> +                  pci_resource_len(pdev, 0),
>>>>>> +                  DRV_NAME)) {
>>>>>
>>>>> Actually, it looks like you are failing to set err from these *m
>>>>> calls, like what
>>>>> you fixed in patch 3. Can you address this for v2?
>>>>
>>>> It returns NULL on error, there is no error code I can set.
>>>
>>> So it does. A quick grep shows that most drivers return -EBUSY.
>>
>> Sure, I just meant that there is no error code I can pass on. I can change
>> that to -EBUSY if you prefer that, currently it just returns -ENODEV as the
>> old code has done before.
> 
> Ping?

I think -EBUSY is a good return here.

I have a WIP at [1] of some logging cleanups on top of your commits.

--Sean

[1] https://github.com/Forty-Bot/linux/commits/hme_base