> -----Original Message----- > From: Yuanhan Liu [mailto:yuanhan....@linux.intel.com] > Sent: Friday, April 14, 2017 2:20 PM > To: Tan, Jianfeng > Cc: dev@dpdk.org; olivier.m...@6wind.com; sta...@dpdk.org > Subject: Re: [PATCH] net/virtio-user: fix not working on 32-bit system > > On Fri, Apr 14, 2017 at 05:53:55AM +0000, Tan, Jianfeng wrote: > > > > diff --git a/drivers/net/virtio/virtqueue.h > b/drivers/net/virtio/virtqueue.h > > > > index f9e3736..f43ea70 100644 > > > > --- a/drivers/net/virtio/virtqueue.h > > > > +++ b/drivers/net/virtio/virtqueue.h > > > > @@ -72,7 +72,8 @@ struct rte_mbuf; > > > > * Return the physical address (or virtual address in case of > > > > * virtio-user) of mbuf data buffer. > > > > */ > > > > -#define VIRTIO_MBUF_ADDR(mb, vq) (*(uint64_t *)((uintptr_t)(mb) > + > > > (vq)->offset)) > > > > +#define VIRTIO_MBUF_ADDR(mb, vq) \ > > > > + ((uint64_t)((uintptr_t)(*(void **)((uintptr_t)(mb) + > > > > (vq)->offset)))) > > > > > > The "void **" cast makes it a bit complex (thus hard to read). I think > > > following should work? > > > > Yes, uintptr_t can work. I thought void ** is easier to understand, meaning > a convert to a pointer which pointing to a pointer. > > It's twisted, isn't it? :) > > > I usually use uintptr_t only for converter from pointer to integer, not the > opposite way. > > Yes, that's a typical usage. But the fact of the matter is uintptr_t > represents the word size, which is exactly what needed in this case.
Another fold, if you refer to the definition of struct rte_mbuf, the first field is defined as void * instead of uintptr_t. I think that why I prefer to use ((void *)*) in the beginning. Thanks, Jianfeng > > --yliu > > > > > > > > (uint64_t(*(uintptr_t *)((uintptr_t)(mb) + (vq)->offset))) > > > > > > Besides, it deserves a comment. > > > > Will add comment in next version. > > > > Thanks, > > Jianfeng > > > > > > > > --yliu > > > > > > > #else > > > > #define VIRTIO_MBUF_ADDR(mb, vq) ((mb)->buf_physaddr) > > > > #endif > > > > -- > > > > 2.7.4
