On 2/3/26 13:05, Michael S. Tsirkin 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.
>>
>> 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]; // priv not
> initialized, will be done too late in the next line
> struct virtio_can_priv *priv = netdev_priv(ndev); // you see it?
> struct device *dev = &priv->vdev->dev;
> unsigned int len;
> int err;
>
>
> and where is the problem?
The problem is that you use priv here to initialize vq in the line before priv
is initialized.
>
> On the flip size, this guarantees we will not forget to initialize.
Static analysis is your friend.
