Re: [PATCH] netfs: Fix gcc-12 warning by embedding vfs inode in netfs_i_context

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> Side note: I think this could have been done with an unnamed union as
> 
>         struct my_inode {
>                 union {
>                         struct inode            vfs_inode;
>                         struct netfs_inode netfs_inode;
>                 };
>         [...]
> 
> instead, with the rule that 'netfs_inode' always starts with a 'struct inode'.

I'm slightly wary of that, lest struct netfs_inode gets randomised.  I'm not
sure how likely that would be without netfs_inode getting explicitly marked.

> But in a lot of cases you really could do so much better: you *have* a
> "struct netfs_inode" to begin with, but you converted it to just
> "struct inode *", and now you're converting it back.
> 
> Look at that AFS code, for example, where we have afs_vnode_cache() doing
> 
>         return netfs_i_cookie(&vnode->netfs.inode);
> 
> and look how it *had* a netfs structure, and it was passing it to a
> netfs function, but it explicitly passed the WRONG TYPE, so now we've
> lost the type information and it is using that cast to fake it all
> back.

Yeah, I didn't look at those as they didn't cause warnings, but you're right -
those should take struct netfs_inode pointers in some cases, rather than
struct inode.

Note that some functions, such as netfs_readpage() and netfs_readpages() do
need to take struct inode pointers as I'm trying to get the VFS ops to jump
into netfslib and get all the VM interface stuff out of the network
filesystems - the idea being that the network filesystem will provide netfslib
primarily with two functions: do a read op and do a write op.

I'll have a look at your patch in a bit.

Thanks,
David





[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux