Re: [PATCH 3/3] virtio_ring: Use const to annotate read-only pointer params

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Mar 08, 2023 at 09:19:38AM -0500, Feng Liu wrote:
> 
> 
> On 2023-03-08 a.m.9:16, Michael S. Tsirkin wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Wed, Mar 08, 2023 at 09:07:49AM -0500, Feng Liu wrote:
> > > 
> > > 
> > > On 2023-03-08 a.m.12:58, Jason Wang wrote:
> > > > External email: Use caution opening links or attachments
> > > > 
> > > > 
> > > > On Tue, Mar 7, 2023 at 11:57 AM Feng Liu <feliu@nvidia.com> wrote:
> > > > > 
> > > > > Add const to make the read-only pointer parameters clear, similar to
> > > > > many existing functions.
> > > > > 
> > > > > Signed-off-by: Feng Liu <feliu@nvidia.com>
> > > > > Reviewed-by: Jiri Pirko <jiri@nvidia.com>
> > > > > Reviewed-by: Parav Pandit <parav@nvidia.com>
> > > > > Reviewed-by: Gavin Li <gavinl@nvidia.com>
> > > > > Reviewed-by: Bodong Wang <bodong@nvidia.com>
> > > > > ---
> > > > >    drivers/virtio/virtio_ring.c | 25 ++++++++++++-------------
> > > > >    include/linux/virtio.h       | 12 ++++++------
> > > > >    2 files changed, 18 insertions(+), 19 deletions(-)
> > > > > 
> > > > 
> > > > [...]
> > > > 
> > > > > 
> > > > > -/*
> > > > > - * This should prevent the device from being used, allowing drivers to
> > > > > +/ This should prevent the device from being used, allowing drivers to
> > > > >     * recover.  You may need to grab appropriate locks to flush.
> > > > >     */
> > > > 
> > > > Any reason for this change?
> > > > 
> > > Hi, Jason
> > > The original comment of the code had a syntax problem and couldn't compile,
> > > I fixed it here
> > 
> > This is how it looked before your patch:
> > 
> > /*
> >   * This should prevent the device from being used, allowing drivers to
> >   * recover.  You may need to grab appropriate locks to flush.
> >   */
> > 
> > I see no problem here.
> > 
> Yes, you are right. I made a mistake here, I will fix it

Nice but the bigger problem is not the mistake - it is the posting of
untested code. It might be an ok thing to do - as long as you make it
super abundantrly clear that this is what it is, and explain why
you are posting it now and not after testing.

> > 
> > > > Thanks
> > > > 
> > 

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