On Thu, Aug 06, 2020 at 11:37:38AM +0800, Jason Wang wrote: > > On 2020/8/5 下午7:45, Michael S. Tsirkin wrote: > > > > #define virtio_cread(vdev, structname, member, ptr) \ > > > > do { \ > > > > might_sleep(); \ > > > > /* Must match the member's type, and be integer */ \ > > > > - if (!typecheck(typeof((((structname*)0)->member)), *(ptr))) \ > > > > + if (!__virtio_typecheck(structname, member, *(ptr))) \ > > > > (*ptr) = 1; \ > > > A silly question, compare to using set()/get() directly, what's the value > > > of the accessors macro here? > > > > > > Thanks > > get/set don't convert to the native endian, I guess that's why > > drivers use cread/cwrite. It is also nice that there's type > > safety, checking the correct integer width is used. > > > Yes, but this is simply because a macro is used here, how about just doing > things similar like virtio_cread_bytes(): > > static inline void virtio_cread(struct virtio_device *vdev, > unsigned int offset, > void *buf, size_t len) > > > And do the endian conversion inside? > > Thanks > Then you lose type safety. It's very easy to have an le32 field and try to read it into a u16 by mistake. These macros are all about preventing bugs: and the whole patchset is about several bugs sparse found - that is what prompted me to make type checks more strict. > > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization