Re: Compiler warnings in FCoE code

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

 



On Mon, Jun 3, 2019 at 2:20 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> In fact, what people *are* passing that thing is this:
>
>         struct {
>                 struct fc_rport_priv rdata;
>                 struct fcoe_rport frport;
>         } buf;

It is in fact worse than that.

Yes, _some_ people pass that combined struct.

But a lot of people pass around a fc_rport_priv that has just been
allocated with

        rdata = kzalloc(sizeof(*rdata) + lport->rport_priv_size, GFP_KERNEL);

in fc_rport_create().

They end up being somewhat equivalent as long as the alignments of
those sub-structures match, but it does mean that the type is really a
bit fluid, and it ends up leaking outside of that fcoe_ctlr.c file
into various other helper functions like that allocation function.

It really looks fairly nasty to change/fix. The code really is passing
around a 'struct fc_rport_priv' pointer, but that that 'fc_rport_priv'
type is then associated with the added 'struct fcoe_rport' at the end,
in a way that is *not* visible to the type system.

So no, it doesn't work to create a new type like

 struct fc_rport_combined_data {

because it ends up affecting almost *everything* that works with that
'rdata' thing.

I get the feeling that 'struct fc_rport_priv' should just be extended
to have 'struct fcoe_rport' at the end, but maybe that's not always
true? It looks like that is only used for FIP_MODE_VN2VN, adn then we
have some other mode that doesn't have that allocation at all?

So the code seems to be a mix of dynamic typing ("no fcoe_rport at the
end") and static typing that just assumes that the allocation always
has the fcoe_rport afterwards.

Would it be ok to make 'struct fc_rport_priv' just always contain that
fcoe_rport? Getting rid of the "lport->rport_priv_size" thing
entirely, and just have the type set to the bigger case
unconditionally?

                  Linus



[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux