Re: [PATCH 3/3] nfsd: clients don't need to break their own delegations

[NeilBrown <neilb@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 1334 bytes --]

On Thu, Sep 07 2017, J. Bruce Fields wrote:

> On Mon, Aug 28, 2017 at 02:32:53PM +1000, NeilBrown wrote:
>> /* legacy typedef, should eventually be removed */
>> typedef void *fl_owner_t;
>> 
>> 
>> Maybe you could do the world a favor and remove fl_owner_t in a
>> preliminary patch :-)
>
> Partly scripted, still a bit tedious, but I think it's right.  Honestly
> I don't know what the motivation for the comment was, though.  Are there
> no documentation or type-checking benefits to having the typdef?

If it was an established practice throughout the kernel to use typedefs
to differentiate different 'void *', then maybe there would be a
documentation benefit.  Given the wide use of casts (you removed 9 I
think), I don't think there are significant type-checking benefits.

I don't like fl_owner_t because when you see it in the general context
of the kernel, you are likely to think that it means something
important.  Then you go hunting and find "Oh, it is just a void*". (That
is what happened to me:-).  The second reason that I don't like it is
that it requires all those casts that you removed.

Reviewed-by: NeilBrown <neilb@suse.com>

Thanks,
NeilBrown


>
> The main annoyance was having this defined as a files_struct pointer,
> which Christoph fixed some time ago.
>
> --b.
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]