On 7/22/19 1:50 PM, Christoph Hellwig wrote:
> On Mon, Jul 22, 2019 at 08:22:30AM +0200, Hannes Reinecke wrote:
>> Gcc-9 complains for a memset across pointer boundaries, which happens
>> as the code tries to allocate a flexible array on the stack.
>> Turns out we cannot do this without relying on gcc-isms, so
>> with this patch we'll embed the fc_rport_priv structure into
>> fcoe_rport, can use the normal 'container_of' outcast, and
>> will only have to do a memset over one structure.
>
> This looks mostly good, but:
>
> I think the subject and changelog are a bit odd. What you do here
> is to change that way how the private data is allocated by embeddeding
> the fc_rport_priv structure into the private data, so that should be
> the focus of the description. That this was triggered by the memset
> warning might be worth mentioning, but probably only after explaining
> what you did.
>
Yeah, that was carried over from the original patch.
Will be updating it.
>> @@ -2738,10 +2736,7 @@ static int fcoe_ctlr_vn_recv(struct fcoe_ctlr *fip,
>> struct sk_buff *skb)
>> {
>> struct fip_header *fiph;
>> enum fip_vn2vn_subcode sub;
>> - struct {
>> - struct fc_rport_priv rdata;
>> - struct fcoe_rport frport;
>> - } buf;
>> + struct fcoe_rport buf;
>
> Wouldn't rport or frport be a better name for this variable?
>
Possibly ...
Keeping it at 'buf' introduced less churn, but by all means...
>> fiph = (struct fip_header *)skb->data;
>> @@ -2757,7 +2752,8 @@ static int fcoe_ctlr_vn_recv(struct fcoe_ctlr *fip,
>> struct sk_buff *skb)
>> goto drop;
>> }
>>
>> - rc = fcoe_ctlr_vn_parse(fip, skb, &buf.rdata);
>> + memset(&buf, 0, sizeof(buf));
>
> Instead of the memset you could do an initialization at declaration
> time:
>
> struct fcoe_rport rport = { };
>
Would probably optimized to the same assembler code by the compiler, but
okay.
Will be reposting.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
