Feng Liu
2023-Mar-08 14:19 UTC
[PATCH 3/3] virtio_ring: Use const to annotate read-only pointer params
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 at nvidia.com> wrote: >>>> >>>> Add const to make the read-only pointer parameters clear, similar to >>>> many existing functions. >>>> >>>> Signed-off-by: Feng Liu <feliu at nvidia.com> >>>> Reviewed-by: Jiri Pirko <jiri at nvidia.com> >>>> Reviewed-by: Parav Pandit <parav at nvidia.com> >>>> Reviewed-by: Gavin Li <gavinl at nvidia.com> >>>> Reviewed-by: Bodong Wang <bodong at 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> >>> Thanks >>> >
Michael S. Tsirkin
2023-Mar-08 14:28 UTC
[PATCH 3/3] virtio_ring: Use const to annotate read-only pointer params
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 at nvidia.com> wrote: > > > > > > > > > > Add const to make the read-only pointer parameters clear, similar to > > > > > many existing functions. > > > > > > > > > > Signed-off-by: Feng Liu <feliu at nvidia.com> > > > > > Reviewed-by: Jiri Pirko <jiri at nvidia.com> > > > > > Reviewed-by: Parav Pandit <parav at nvidia.com> > > > > > Reviewed-by: Gavin Li <gavinl at nvidia.com> > > > > > Reviewed-by: Bodong Wang <bodong at 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 itNice 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 > > > > > >