Re: [PATCH v2] virtio_blk: add VIRTIO_BLK_F_LIFETIME feature support

From: Alvaro Karsz <alvaro.karsz@solid-run.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jens Axboe <axboe@kernel.dk>, Paolo Bonzini <pbonzini@redhat.com>,
	Chaitanya Kulkarni <chaitanyak@nvidia.com>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	"virtualization@lists.linux-foundation.org"
	<virtualization@lists.linux-foundation.org>
Subject: Re: [PATCH v2] virtio_blk: add VIRTIO_BLK_F_LIFETIME feature support
Date: Thu, 24 Nov 2022 10:07:46 +0200	[thread overview]
Message-ID: <CAJs=3_DJzeGV4KEtTdXAo2SaSQh5UiuT9+0Kvqq3exbs2ZYBLQ@mail.gmail.com> (raw)
In-Reply-To: <20221124014300-mutt-send-email-mst@kernel.org>

Thanks for your comments.

> > +     /* Pass the data to the user */
> > +     if (copy_to_user((void __user *)arg, &lifetime, sizeof(lifetime))) {
> > +             ret = -EFAULT;
> > +             goto out;
>
>
> there nothing here to "goto out" following is sufficient I guess :-

You're right, but like Michael said, it will be pretty easy to forget
adding a goto statement if adding more code to the function.

> > +     memset(&lifetime, 0, sizeof(lifetime));
> > +
>
>
> you can remove memset 0 call here and declare initialize struct var
> something like totally untested :-
>
>
>         struct virtio_blk_lifetime lifetime = { };

>
> Yes, that's a bit cleaner, but there should be no space between {}:
>         struct virtio_blk_lifetime lifetime = {};

I will fix it in the next version.

Alvaro
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization