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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2023-03-08 a.m.11:49, Michael S. Tsirkin wrote:
External email: Use caution opening links or attachments


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@xxxxxxxxxxxxxxxxxxxxxxxxxx>
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.


Hi,Michael
   COOL! Sounds good! Will do that



--
MST

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization



[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux