Re: [PATCH] virtio: Change typecasts in vring_init()

On Fri, Aug 30, 2019 at 05:58:23PM +0000, Matej Genci wrote:

On 8/30/2019 3:02 PM, Michael S. Tsirkin wrote:

quoted

On Tue, Aug 27, 2019 at 03:20:57PM +0000, Matej Genci wrote:

quoted

Compilers such as g++ 7.3 complain about assigning void* variable to
a non-void* variable (like struct pointers) and pointer arithmetics
on void*.

Signed-off-by: Matej Genci <redacted>
---
  include/uapi/linux/virtio_ring.h | 9 +++++----
  1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 4c4e24c291a5..2c339b9e2923 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h

@@ -168,10 +168,11 @@ static inline void vring_init(struct vring *vr, unsigned int num, void *p,
  			      unsigned long align)
  {
  	vr->num = num;
-	vr->desc = p;
-	vr->avail = p + num*sizeof(struct vring_desc);
-	vr->used = (void *)(((uintptr_t)&vr->avail->ring[num] + sizeof(__virtio16)
-		+ align-1) & ~(align - 1));
+	vr->desc = (struct vring_desc *)p;
+	vr->avail = (struct vring_avail *)((uintptr_t)p
+		+ num*sizeof(struct vring_desc));
+	vr->used = (struct vring_used *)(((uintptr_t)&vr->avail->ring[num]
+		+ sizeof(__virtio16) + align-1) & ~(align - 1));
  }
  
  static inline unsigned vring_size(unsigned int num, unsigned long align)

I'm not really interested in building with g++, sorry.
Centainly not if it makes code less robust by forcing
casts where they weren't previously necessary.

Can you elaborate on how these casts make the code less robust?

If we ever change the variable types build will still pass
because of the cast.

They aren't necessary in C but I think being explicit can improve
readability as argued in
https://softwareengineering.stackexchange.com/a/275714

quoted

However, vring_init and vring_size are legacy APIs anyway,
so I'd be happy to add ifndefs that will allow userspace
simply hide these functions if it does not need them.

I feel like my patch is a harmless way of allowing this header
to be used in C++ projects, but I'm happy to drop it in lieu of
the guards if you feel strongly about it.

Thanks.
Matej

Yea let's not even start.

quoted

quoted

-- 
2.22.0