Michael S. Tsirkin
2023-Mar-08 16:25 UTC
[PATCH 3/3] virtio_ring: Use const to annotate read-only pointer params
On Wed, Mar 08, 2023 at 10:59:57AM -0500, Feng Liu wrote:> > > On 2023-03-08 a.m.9:13, Michael S. Tsirkin wrote: > > External email: Use caution opening links or attachments > > > > > > On Tue, Mar 07, 2023 at 09:17:55PM +0000, Feng Liu wrote: > > > On 2023-03-07 04:14, David Edmondson wrote: > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > Feng Liu via Virtualization <virtualization at lists.linux-foundation.org> > > > > writes: > > > > > > > > > Add const to make the read-only pointer parameters clear, similar to > > > > > many existing functions. > > > > > > > > In many of the modified functions the local variable that is a cast of > > > > the argument could also be const. Is there a reason not to do both at > > > > the same time? > > > > > > > > > > Hi,David > > > > > > In order to prevent the content of a pointer parameter from being > > > modified and increase the readability of the function, it is recommended > > > to add the 'const' keyword to the parameter. This is not necessary for > > > local variables and non-pointer parameters, as they are only stored on > > > the stack and do not affect the original value or structure member > > > passed into the function. Therefore, in this case, the 'const' keyword > > > is only added to pointer parameters. > > > > This makes no sense to me. If ytou cast away the const then it is > > pointless. > > > > Hi, Michael > > I really don't quite understand your point of view. > Is a local variable that needs to be add const? Can you help to point > out the specific problem/point ?I just repeated what David said. Basically most of these functions use to_vvq which uses container_of which in turn loses const qualifier. So your change is poinless since rest of code accesses vq through to_vvq. What to do? I don't like the idea of to_vvq_const. So I propose a version of container_of using _Generic which preserves the const qualifier. #define container_of(ptr, type, member) \ ({ \ const void *__mptr = (ptr); \ static_assert(__same_type(*(ptr), ((type *)0)->member) || \ __same_type(*(ptr), void), \ "pointer type mismatch in container_of()"); \ _Generic((ptr), \ typeof(&((const type *)0)->member): \ (const type *)(__mptr - offsetof(type, member)), \ default: \ (type *)(__mptr - offsetof(type, member))); \ }) I'll hack it up in a day or two and post.> > > > > > > > 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(-) > > > > > > > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > > > > > index 806cc44eae64..8010fd1d2047 100644 > > > > > --- a/drivers/virtio/virtio_ring.c > > > > > +++ b/drivers/virtio/virtio_ring.c > > > > > @@ -233,7 +233,7 @@ static void vring_free(struct virtqueue *_vq); > > > > > > > > > > #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq) > > > > > > > > > > -static bool virtqueue_use_indirect(struct vring_virtqueue *vq, > > > > > +static bool virtqueue_use_indirect(const struct vring_virtqueue *vq, > > > > > unsigned int total_sg) > > > > > { > > > > > /* > > > > > @@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq, > > > > > * unconditionally on data path. > > > > > */ > > > > > > > > > > -static bool vring_use_dma_api(struct virtio_device *vdev) > > > > > +static bool vring_use_dma_api(const struct virtio_device *vdev) > > > > > { > > > > > if (!virtio_has_dma_quirk(vdev)) > > > > > return true; > > > > > @@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev) > > > > > return false; > > > > > } > > > > > > > > > > -size_t virtio_max_dma_size(struct virtio_device *vdev) > > > > > +size_t virtio_max_dma_size(const struct virtio_device *vdev) > > > > > { > > > > > size_t max_segment_size = SIZE_MAX; > > > > > > > > > > @@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num) > > > > > */ > > > > > > > > > > static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq, > > > > > - struct vring_desc *desc) > > > > > + const struct vring_desc *desc) > > > > > { > > > > > u16 flags; > > > > > > > > > > @@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx) > > > > > } > > > > > > > > > > static void vring_unmap_extra_packed(const struct vring_virtqueue *vq, > > > > > - struct vring_desc_extra *extra) > > > > > + const struct vring_desc_extra *extra) > > > > > { > > > > > u16 flags; > > > > > > > > > > @@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq, > > > > > } > > > > > > > > > > static void vring_unmap_desc_packed(const struct vring_virtqueue *vq, > > > > > - struct vring_packed_desc *desc) > > > > > + const struct vring_packed_desc *desc) > > > > > { > > > > > u16 flags; > > > > > > > > > > @@ -2786,7 +2786,7 @@ EXPORT_SYMBOL_GPL(vring_transport_features); > > > > > * Returns the size of the vring. This is mainly used for boasting to > > > > > * userspace. Unlike other operations, this need not be serialized. > > > > > */ > > > > > -unsigned int virtqueue_get_vring_size(struct virtqueue *_vq) > > > > > +unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq) > > > > > { > > > > > > > > > > struct vring_virtqueue *vq = to_vvq(_vq); > > > > > @@ -2819,7 +2819,7 @@ void __virtqueue_unbreak(struct virtqueue *_vq) > > > > > } > > > > > EXPORT_SYMBOL_GPL(__virtqueue_unbreak); > > > > > > > > > > -bool virtqueue_is_broken(struct virtqueue *_vq) > > > > > +bool virtqueue_is_broken(const struct virtqueue *_vq) > > > > > { > > > > > struct vring_virtqueue *vq = to_vvq(_vq); > > > > > > > > > > @@ -2827,8 +2827,7 @@ bool virtqueue_is_broken(struct virtqueue *_vq) > > > > > } > > > > > EXPORT_SYMBOL_GPL(virtqueue_is_broken); > > > > > > > > > > -/* > > > > > - * 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. > > > > > */ > > > > > void virtio_break_device(struct virtio_device *dev) > > > > > @@ -2881,7 +2880,7 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq) > > > > > } > > > > > EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr); > > > > > > > > > > -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq) > > > > > +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq) > > > > > { > > > > > struct vring_virtqueue *vq = to_vvq(_vq); > > > > > > > > > > @@ -2895,7 +2894,7 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq) > > > > > } > > > > > EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr); > > > > > > > > > > -dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq) > > > > > +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq) > > > > > { > > > > > struct vring_virtqueue *vq = to_vvq(_vq); > > > > > > > > > > @@ -2910,7 +2909,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq) > > > > > EXPORT_SYMBOL_GPL(virtqueue_get_used_addr); > > > > > > > > > > /* Only available for split ring */ > > > > > -const struct vring *virtqueue_get_vring(struct virtqueue *vq) > > > > > +const struct vring *virtqueue_get_vring(const struct virtqueue *vq) > > > > > { > > > > > return &to_vvq(vq)->split.vring; > > > > > } > > > > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h > > > > > index 2b472514c49b..36a79374e735 100644 > > > > > --- a/include/linux/virtio.h > > > > > +++ b/include/linux/virtio.h > > > > > @@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq); > > > > > > > > > > void *virtqueue_detach_unused_buf(struct virtqueue *vq); > > > > > > > > > > -unsigned int virtqueue_get_vring_size(struct virtqueue *vq); > > > > > +unsigned int virtqueue_get_vring_size(const struct virtqueue *vq); > > > > > > > > > > -bool virtqueue_is_broken(struct virtqueue *vq); > > > > > +bool virtqueue_is_broken(const struct virtqueue *vq); > > > > > > > > > > -const struct vring *virtqueue_get_vring(struct virtqueue *vq); > > > > > -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq); > > > > > -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq); > > > > > -dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq); > > > > > +const struct vring *virtqueue_get_vring(const struct virtqueue *vq); > > > > > +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq); > > > > > +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq); > > > > > +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq); > > > > > > > > > > int virtqueue_resize(struct virtqueue *vq, u32 num, > > > > > void (*recycle)(struct virtqueue *vq, void *buf)); > > > > > -- > > > > > 2.34.1 > > > > > > > > > > _______________________________________________ > > > > > Virtualization mailing list > > > > > Virtualization at lists.linux-foundation.org > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.linuxfoundation.org%2Fmailman%2Flistinfo%2Fvirtualization&data=05%7C01%7Cfeliu%40nvidia.com%7C0f6803f1797f448823ac08db1fdf67e5%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638138816610707030%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=FxggnD7U8o%2B%2BqcYnHN6nc%2BemGVRU1ia5sA4k%2FRTDD7U%3D&reserved=0 > > > > -- > > > > Woke up in my clothes again this morning, don't know exactly where I am. > > > > >
Feng Liu
2023-Mar-08 16:44 UTC
[PATCH 3/3] virtio_ring: Use const to annotate read-only pointer params
On 2023-03-08 a.m.11:25, Michael S. Tsirkin wrote:> External email: Use caution opening links or attachments > > > On Wed, Mar 08, 2023 at 10:59:57AM -0500, Feng Liu wrote: >> >> >> On 2023-03-08 a.m.9:13, Michael S. Tsirkin wrote: >>> External email: Use caution opening links or attachments >>> >>> >>> On Tue, Mar 07, 2023 at 09:17:55PM +0000, Feng Liu wrote: >>>> On 2023-03-07 04:14, David Edmondson wrote: >>>>> External email: Use caution opening links or attachments >>>>> >>>>> >>>>> Feng Liu via Virtualization <virtualization at lists.linux-foundation.org> >>>>> writes: >>>>> >>>>>> Add const to make the read-only pointer parameters clear, similar to >>>>>> many existing functions. >>>>> >>>>> In many of the modified functions the local variable that is a cast of >>>>> the argument could also be const. Is there a reason not to do both at >>>>> the same time? >>>>> >>>> >>>> Hi,David >>>> >>>> In order to prevent the content of a pointer parameter from being >>>> modified and increase the readability of the function, it is recommended >>>> to add the 'const' keyword to the parameter. This is not necessary for >>>> local variables and non-pointer parameters, as they are only stored on >>>> the stack and do not affect the original value or structure member >>>> passed into the function. Therefore, in this case, the 'const' keyword >>>> is only added to pointer parameters. >>> >>> This makes no sense to me. If ytou cast away the const then it is >>> pointless. >>> >> >> Hi, Michael >> >> I really don't quite understand your point of view. >> Is a local variable that needs to be add const? Can you help to point >> out the specific problem/point ? > > I just repeated what David said. Basically most of these functions use > to_vvq which uses container_of which in turn loses const qualifier. > So your change is poinless since rest of code accesses vq through > to_vvq. > > What to do? I don't like the idea of to_vvq_const. > So I propose a version of container_of using _Generic > which preserves the const qualifier. > > > #define container_of(ptr, type, member) \ > ({ \ > const void *__mptr = (ptr); \ > static_assert(__same_type(*(ptr), ((type *)0)->member) || \ > __same_type(*(ptr), void), \ > "pointer type mismatch in container_of()"); \ > _Generic((ptr), \ > typeof(&((const type *)0)->member): \ > (const type *)(__mptr - offsetof(type, member)), \ > default: \ > (type *)(__mptr - offsetof(type, member))); \ > }) > > > I'll hack it up in a day or two and post. > >Hi, Michael Ok, I see. Thanks for your explaination. I will push the next version of this patch after you post> > >>>> >>>>>> 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(-) >>>>>> >>>>>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c >>>>>> index 806cc44eae64..8010fd1d2047 100644 >>>>>> --- a/drivers/virtio/virtio_ring.c >>>>>> +++ b/drivers/virtio/virtio_ring.c >>>>>> @@ -233,7 +233,7 @@ static void vring_free(struct virtqueue *_vq); >>>>>> >>>>>> #define to_vvq(_vq) container_of(_vq, struct vring_virtqueue, vq) >>>>>> >>>>>> -static bool virtqueue_use_indirect(struct vring_virtqueue *vq, >>>>>> +static bool virtqueue_use_indirect(const struct vring_virtqueue *vq, >>>>>> unsigned int total_sg) >>>>>> { >>>>>> /* >>>>>> @@ -269,7 +269,7 @@ static bool virtqueue_use_indirect(struct vring_virtqueue *vq, >>>>>> * unconditionally on data path. >>>>>> */ >>>>>> >>>>>> -static bool vring_use_dma_api(struct virtio_device *vdev) >>>>>> +static bool vring_use_dma_api(const struct virtio_device *vdev) >>>>>> { >>>>>> if (!virtio_has_dma_quirk(vdev)) >>>>>> return true; >>>>>> @@ -289,7 +289,7 @@ static bool vring_use_dma_api(struct virtio_device *vdev) >>>>>> return false; >>>>>> } >>>>>> >>>>>> -size_t virtio_max_dma_size(struct virtio_device *vdev) >>>>>> +size_t virtio_max_dma_size(const struct virtio_device *vdev) >>>>>> { >>>>>> size_t max_segment_size = SIZE_MAX; >>>>>> >>>>>> @@ -423,7 +423,7 @@ static void virtqueue_init(struct vring_virtqueue *vq, u32 num) >>>>>> */ >>>>>> >>>>>> static void vring_unmap_one_split_indirect(const struct vring_virtqueue *vq, >>>>>> - struct vring_desc *desc) >>>>>> + const struct vring_desc *desc) >>>>>> { >>>>>> u16 flags; >>>>>> >>>>>> @@ -1183,7 +1183,7 @@ static u16 packed_last_used(u16 last_used_idx) >>>>>> } >>>>>> >>>>>> static void vring_unmap_extra_packed(const struct vring_virtqueue *vq, >>>>>> - struct vring_desc_extra *extra) >>>>>> + const struct vring_desc_extra *extra) >>>>>> { >>>>>> u16 flags; >>>>>> >>>>>> @@ -1206,7 +1206,7 @@ static void vring_unmap_extra_packed(const struct vring_virtqueue *vq, >>>>>> } >>>>>> >>>>>> static void vring_unmap_desc_packed(const struct vring_virtqueue *vq, >>>>>> - struct vring_packed_desc *desc) >>>>>> + const struct vring_packed_desc *desc) >>>>>> { >>>>>> u16 flags; >>>>>> >>>>>> @@ -2786,7 +2786,7 @@ EXPORT_SYMBOL_GPL(vring_transport_features); >>>>>> * Returns the size of the vring. This is mainly used for boasting to >>>>>> * userspace. Unlike other operations, this need not be serialized. >>>>>> */ >>>>>> -unsigned int virtqueue_get_vring_size(struct virtqueue *_vq) >>>>>> +unsigned int virtqueue_get_vring_size(const struct virtqueue *_vq) >>>>>> { >>>>>> >>>>>> struct vring_virtqueue *vq = to_vvq(_vq); >>>>>> @@ -2819,7 +2819,7 @@ void __virtqueue_unbreak(struct virtqueue *_vq) >>>>>> } >>>>>> EXPORT_SYMBOL_GPL(__virtqueue_unbreak); >>>>>> >>>>>> -bool virtqueue_is_broken(struct virtqueue *_vq) >>>>>> +bool virtqueue_is_broken(const struct virtqueue *_vq) >>>>>> { >>>>>> struct vring_virtqueue *vq = to_vvq(_vq); >>>>>> >>>>>> @@ -2827,8 +2827,7 @@ bool virtqueue_is_broken(struct virtqueue *_vq) >>>>>> } >>>>>> EXPORT_SYMBOL_GPL(virtqueue_is_broken); >>>>>> >>>>>> -/* >>>>>> - * 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. >>>>>> */ >>>>>> void virtio_break_device(struct virtio_device *dev) >>>>>> @@ -2881,7 +2880,7 @@ dma_addr_t virtqueue_get_desc_addr(struct virtqueue *_vq) >>>>>> } >>>>>> EXPORT_SYMBOL_GPL(virtqueue_get_desc_addr); >>>>>> >>>>>> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq) >>>>>> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *_vq) >>>>>> { >>>>>> struct vring_virtqueue *vq = to_vvq(_vq); >>>>>> >>>>>> @@ -2895,7 +2894,7 @@ dma_addr_t virtqueue_get_avail_addr(struct virtqueue *_vq) >>>>>> } >>>>>> EXPORT_SYMBOL_GPL(virtqueue_get_avail_addr); >>>>>> >>>>>> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq) >>>>>> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *_vq) >>>>>> { >>>>>> struct vring_virtqueue *vq = to_vvq(_vq); >>>>>> >>>>>> @@ -2910,7 +2909,7 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *_vq) >>>>>> EXPORT_SYMBOL_GPL(virtqueue_get_used_addr); >>>>>> >>>>>> /* Only available for split ring */ >>>>>> -const struct vring *virtqueue_get_vring(struct virtqueue *vq) >>>>>> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq) >>>>>> { >>>>>> return &to_vvq(vq)->split.vring; >>>>>> } >>>>>> diff --git a/include/linux/virtio.h b/include/linux/virtio.h >>>>>> index 2b472514c49b..36a79374e735 100644 >>>>>> --- a/include/linux/virtio.h >>>>>> +++ b/include/linux/virtio.h >>>>>> @@ -84,14 +84,14 @@ bool virtqueue_enable_cb_delayed(struct virtqueue *vq); >>>>>> >>>>>> void *virtqueue_detach_unused_buf(struct virtqueue *vq); >>>>>> >>>>>> -unsigned int virtqueue_get_vring_size(struct virtqueue *vq); >>>>>> +unsigned int virtqueue_get_vring_size(const struct virtqueue *vq); >>>>>> >>>>>> -bool virtqueue_is_broken(struct virtqueue *vq); >>>>>> +bool virtqueue_is_broken(const struct virtqueue *vq); >>>>>> >>>>>> -const struct vring *virtqueue_get_vring(struct virtqueue *vq); >>>>>> -dma_addr_t virtqueue_get_desc_addr(struct virtqueue *vq); >>>>>> -dma_addr_t virtqueue_get_avail_addr(struct virtqueue *vq); >>>>>> -dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq); >>>>>> +const struct vring *virtqueue_get_vring(const struct virtqueue *vq); >>>>>> +dma_addr_t virtqueue_get_desc_addr(const struct virtqueue *vq); >>>>>> +dma_addr_t virtqueue_get_avail_addr(const struct virtqueue *vq); >>>>>> +dma_addr_t virtqueue_get_used_addr(const struct virtqueue *vq); >>>>>> >>>>>> int virtqueue_resize(struct virtqueue *vq, u32 num, >>>>>> void (*recycle)(struct virtqueue *vq, void *buf)); >>>>>> -- >>>>>> 2.34.1 >>>>>> >>>>>> _______________________________________________ >>>>>> Virtualization mailing list >>>>>> Virtualization at lists.linux-foundation.org >>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.linuxfoundation.org%2Fmailman%2Flistinfo%2Fvirtualization&data=05%7C01%7Cfeliu%40nvidia.com%7C7dc87cb48ac44bead13808db1ff1d29a%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C638138895717817297%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=ZT5o%2Fs2TWQT6yP7tUUDnV4ojKgKtUmEzSVpnDTs5PtU%3D&reserved=0 >>>>> -- >>>>> Woke up in my clothes again this morning, don't know exactly where I am. >>>> >>> >
Michael S. Tsirkin
2023-Mar-08 16:49 UTC
[PATCH 3/3] virtio_ring: Use const to annotate read-only pointer params
On Wed, Mar 08, 2023 at 11:26:04AM -0500, Michael S. Tsirkin wrote:> On Wed, Mar 08, 2023 at 10:59:57AM -0500, Feng Liu wrote: > > > > > > On 2023-03-08 a.m.9:13, Michael S. Tsirkin wrote: > > > External email: Use caution opening links or attachments > > > > > > > > > On Tue, Mar 07, 2023 at 09:17:55PM +0000, Feng Liu wrote: > > > > On 2023-03-07 04:14, David Edmondson wrote: > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > > > > Feng Liu via Virtualization <virtualization at lists.linux-foundation.org> > > > > > writes: > > > > > > > > > > > Add const to make the read-only pointer parameters clear, similar to > > > > > > many existing functions. > > > > > > > > > > In many of the modified functions the local variable that is a cast of > > > > > the argument could also be const. Is there a reason not to do both at > > > > > the same time? > > > > > > > > > > > > > Hi,David > > > > > > > > In order to prevent the content of a pointer parameter from being > > > > modified and increase the readability of the function, it is recommended > > > > to add the 'const' keyword to the parameter. This is not necessary for > > > > local variables and non-pointer parameters, as they are only stored on > > > > the stack and do not affect the original value or structure member > > > > passed into the function. Therefore, in this case, the 'const' keyword > > > > is only added to pointer parameters. > > > > > > This makes no sense to me. If ytou cast away the const then it is > > > pointless. > > > > > > > Hi, Michael > > > > I really don't quite understand your point of view. > > Is a local variable that needs to be add const? Can you help to point > > out the specific problem/point ? > > I just repeated what David said. Basically most of these functions use > to_vvq which uses container_of which in turn loses const qualifier. > So your change is poinless since rest of code accesses vq through > to_vvq. > > What to do? I don't like the idea of to_vvq_const. > So I propose a version of container_of using _Generic > which preserves the const qualifier. > > > #define container_of(ptr, type, member) \ > ({ \ > const void *__mptr = (ptr); \ > static_assert(__same_type(*(ptr), ((type *)0)->member) || \ > __same_type(*(ptr), void), \ > "pointer type mismatch in container_of()"); \ > _Generic((ptr), \ > typeof(&((const type *)0)->member): \ > (const type *)(__mptr - offsetof(type, member)), \ > default: \ > (type *)(__mptr - offsetof(type, member))); \ > }) > > > I'll hack it up in a day or two and post. >Oh wait a second. There's already container_of_const. So just use it in to_vvq. -- MST