Marc Kleine-Budde wrote:
> Oliver Hartkopp wrote:
>>>> + *cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
>> This could be also the place, where we could memset the struct can_frame to
>> zero, right?
>
>> Maybe we should do it here and leave it in alloc_can_err_skb() ...
>
> In my at91 driver I don't need the memset, because I unconditionally
> read the two 32 bit registers and write it into the ->data of the can_frame.
>
>>>> +
>>>> + return skb;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(alloc_can_skb);
>>>> +struct sk_buff *alloc_can_err_skb(struct net_device *dev, struct
>>>> can_frame **cf)
>>>> +{
>>>> + struct sk_buff *skb;
>>>> +
>>>> + skb = alloc_can_skb(dev, cf);
>>>> + if (unlikely(!skb))
>>>> + return NULL;
>>>> +
>>>> + memset(*cf, 0, sizeof(struct can_frame));
>> here :-)
>
> Here we need zeroed ->data
Ok, but if we think about some improvement, i would suggest to zero only the 8
bytes data[] and leave the rest as-is.
Maybe by setting the u64 casted data[] to zero.
>
>>>> + (*cf)->can_id = CAN_ERR_FLAG;
>>>> + (*cf)->can_dlc = CAN_ERR_DLC;
As you can see, the can_id and the can_dlc is written anyway.
When data[] is uninitialized, we expose 8-11 Bytes of uninitialized contiguous
data to the userspace, when can_dlc=0 - i don't know if this is relevant.
Regards,
Oliver
_______________________________________________
Socketcan-core mailing list
[email protected]
https://lists.berlios.de/mailman/listinfo/socketcan-core
