On Tue, Oct 14, 2025 at 08:54:23AM +0200, Francesco Valla wrote: > On Monday, 13 October 2025 at 18:35:51 Matias Ezequiel Vara Larsen > <[email protected]> wrote: > > On Thu, Sep 11, 2025 at 10:59:40PM +0200, Francesco Valla wrote: > > > [...] > > > > > > > + > > > > +/* TX queue message types */ > > > > +struct virtio_can_tx_out { > > > > +#define VIRTIO_CAN_TX 0x0001 > > > > + __le16 msg_type; > > > > + __le16 length; /* 0..8 CC, 0..64 CAN-FD, 0..2048 CAN-XL, 12 > > > > bits */ > > > > + __u8 reserved_classic_dlc; /* If CAN classic length = 8 then > > > > DLC can be 8..15 */ > > > > + __u8 padding; > > > > + __le16 reserved_xl_priority; /* May be needed for CAN XL > > > > priority */ > > > > + __le32 flags; > > > > + __le32 can_id; > > > > + __u8 sdu[64]; > > > > +}; > > > > + > > > > > > sdu[] here might be a flexible array, if the driver allocates > > > virtio_can_tx_out structs dyncamically (see above). This would be > > > beneficial in case of CAN-XL frames (if/when they will be supported). > > > > > > > If we use a flexible array for sdu[] here, then we will have a problem > > when defining the virtio_can_tx struct since it is not in the end of the > > structure. I think it is a good idea to define it as a flexible array > > but I do not know how. > > In this case, I'd move struct virtio_can_tx_out at the end of the > virtio_can_tx struct - in this way, sdu[] would be at the end: > > struct virtio_can_tx { > struct list_head list; > unsigned int putidx; > struct virtio_can_tx_in tx_in; > struct virtio_can_tx_out tx_out; > }; >
Done. > Maybe an additional comment declaring why it is done this way would > be a good idea? Also considering that the two structures are defined > in different files. > I am not sure if a comment is required since moving the tx_out field would make the compiler complains anyway but I do not have an strong opinion. Also, would it help to put both structures in the same file? Matias
