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
