On Tue, Feb 03, 2026 at 01:32:47PM +0100, Vincent Mailhol wrote: > On Mar. 3 Feb. 2026 at 13:05, Michael S. Tsirkin <[email protected]> wrote: > > On Tue, Feb 03, 2026 at 12:55:07PM +0100, Harald Mommer wrote: > > > > > > > > > On 1/9/26 18:23, Francesco Valla wrote: > > > >> +static u8 virtio_can_send_ctrl_msg(struct net_device *ndev, u16 > > > >> msg_type) > > > >> +{ > > > >> + struct scatterlist sg_out, sg_in, *sgs[2] = { &sg_out, &sg_in }; > > > >> + struct virtio_can_priv *priv = netdev_priv(ndev); > > > >> + struct device *dev = &priv->vdev->dev; > > > >> + struct virtqueue *vq; > > > >> + unsigned int len; > > > >> + int err; > > > >> + > > > >> + vq = priv->vqs[VIRTIO_CAN_QUEUE_CONTROL]; > > > > Nit: consider initializing this above, while declaring it. > > > > > > All those "Nit" regarding initialization cause problems. There is a > > > reason why it was done the way it is. > > > > > > The network people require that the declaration lines are ordered by line > > > length. longest line first. This is called "Reverse Christmas tree". > > > Don't ask me why, this formatting style is what the network people > > > require. Their subsystem, their rules. > > I am fine with the Reverse Christmas Tree in general, except when it > randomly splits the initialization, as is the case here. As you noted, > this is a coding rule of the network subsystem, but here we are the > CAN subsystem. So yes, the CAN is itself a sub-subsystem of network, > but my point is that we are a different team of maintainers. I would > like to ask the network maintainers for understanding regarding our > different preferences on that topic. > > Unless Marc or Oliver have a strong opinion on this, I would prefer > not to push the Reverse Christmas Tree to its limits and to allow, > within the CAN subtree, exceptions whenever this would avoid some > hanging initializations. > > > > To initialize the vq you need now already the priv initialized. If now > > > the vq line becomes longer than the priv line you will violate the > > > special formatting requirements of the network subsystem. > > > > > > Solution was: What you see above. > > > > > > Regards > > > Harald > > > > So you reorder it then: > > > > struct scatterlist sg_out, sg_in, *sgs[2] = { &sg_out, &sg_in }; > > struct virtqueue *vq = priv->vqs[VIRTIO_CAN_QUEUE_CONTROL]; > > struct virtio_can_priv *priv = netdev_priv(ndev); > > struct device *dev = &priv->vdev->dev; > > unsigned int len; > > int err; > > > > > > and where is the problem? > > The problem is that priv is not yet declared. So this: > > struct scatterlist sg_out, sg_in, *sgs[2] = { &sg_out, &sg_in }; > struct virtio_can_priv *priv = netdev_priv(ndev); > struct virtqueue *vq = priv->vqs[VIRTIO_CAN_QUEUE_CONTROL]; > struct device *dev = &priv->vdev->dev; > unsigned int len; > int err; > > is forced,
Ah. Good point. > which, IMHO, is totally *fine* and way better than > deporting down the vq initialization. Indeed. > > On the flip size, this guarantees we will not forget to initialize. > > Yes! > > > Yours sincerely, > Vincent Mailhol
