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  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]
Date:   Wed, 27 Feb 2019 14:45:44 +0100
From:   Frederic Barrat <fbarrat@...ux.ibm.com>
To:     Andrew Donnellan <andrew.donnellan@....ibm.com>,
        "Alastair D'Silva" <alastair@...ilva.org>,
        "'Alastair D'Silva'" <alastair@....ibm.com>
Cc:     "'Greg Kurz'" <groug@...d.org>, "'Arnd Bergmann'" <arnd@...db.de>,
        "'Greg Kroah-Hartman'" <gregkh@...uxfoundation.org>,
        linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/5] ocxl: Rename struct link to ocxl_link



Le 27/02/2019 à 09:18, Andrew Donnellan a écrit :
> On 27/2/19 7:04 pm, Alastair D'Silva wrote:
>>> -----Original Message-----
>>> From: Andrew Donnellan <andrew.donnellan@....ibm.com>
>>> Sent: Wednesday, 27 February 2019 6:55 PM
>>> To: Alastair D'Silva <alastair@...ilva.org>; 'Alastair D'Silva'
>>> <alastair@....ibm.com>
>>> Cc: 'Greg Kurz' <groug@...d.org>; 'Frederic Barrat'
>>> <fbarrat@...ux.ibm.com>; 'Arnd Bergmann' <arnd@...db.de>; 'Greg Kroah-
>>> Hartman' <gregkh@...uxfoundation.org>; linuxppc-dev@...ts.ozlabs.org;
>>> linux-kernel@...r.kernel.org
>>> Subject: Re: [PATCH 1/5] ocxl: Rename struct link to ocxl_link
>>>
>>> On 27/2/19 6:34 pm, Alastair D'Silva wrote:>>> diff --git
>>> a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c index
>>>>>> e6a607488f8a..16eb8a60d5c7 100644
>>>>>> --- a/drivers/misc/ocxl/file.c
>>>>>> +++ b/drivers/misc/ocxl/file.c
>>>>>> @@ -152,7 +152,7 @@ static long afu_ioctl_enable_p9_wait(struct
>>>>>> ocxl_context *ctx,
>>>>>>
>>>>>>             if (status == ATTACHED) {
>>>>>>                 int rc;
>>>>>> -            struct link *link = ctx->afu->fn->link;
>>>>>> +            void *link = ctx->afu->fn->link;
>>>>>
>>>>> This doesn't look like a rename...
>>>>
>>>> That corrects the type to what the member (and prototype for
>>> ocxl_link_update_pe) declare it as.
>>>>
>>>> The struct link there is bogus, it shouldn't even compile (since the 
>>>> intended
>>> struct link is defined in a different compilation unit), but instead 
>>> picks up a
>>> different definition of 'struct link' from elsewhere.
>>>>
>>>
>>> Given there's only a handful of struct links defined across the 
>>> entire kernel,
>>> I'm going to guess that the definition it's picking up is in fact the 
>>> ocxl one.
>>>
>>
>> Unlikely, since that's never in a header. It wasn't caught since it 
>> was assigned to/from a void*.
> 
> Ah, yeah that'd explain it... and it's a pointer so it never needs to 
> know its size. I'm clearly not very good at C.
> 
>>
>>> I think the better solution here is to move struct ocxl_link into
>>> ocxl_internal.h, change ocxl_fn::link to be struct ocxl_link * rather 
>>> than void
>>> *, and update the function signature for ocxl_link_update_pe() as well.
>> Not move it, but we could have an opaque declaration there.
>>
> 
> Putting it there would fit with all the other ocxl_* structs, but either 
> way, we definitely need a declaration in there and get rid of the void*, t


Mmm, it might turn out to be more invasive that planned...
The point was only to have it as an opaque to the outside world, for 
APIs we'd like to deprecate at some point, so I wouldn't sweat too much 
over it.

   Fred

Powered by blists - more mailing lists