Re: [Xen-devel] [PATCH v3 1/3] ring.h: introduce macros to handle monodirectional rings with multiple req sizes

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 23 Mar 2017, Stefano Stabellini wrote:
> CC'ing Jan
> 
> On Tue, 21 Mar 2017, Stefano Stabellini wrote:
> > +static inline void name##_read_packet(const unsigned char *buf,            
> >    \
> > +        RING_IDX masked_prod, RING_IDX *masked_cons,                       
> >    \
> > +        RING_IDX ring_size, void *opaque, size_t size)                     
> >    \
> > +{                                                                          
> >    \
> > +    if (*masked_cons < masked_prod ||                                      
> >    \
> > +            size <= ring_size - *masked_cons) {                            
> >    \
> > +        memcpy(opaque, buf + *masked_cons, size);                          
> >    \
> > +    } else {                                                               
> >    \
> > +        memcpy(opaque, buf + *masked_cons, ring_size - *masked_cons);      
> >    \
> > +        memcpy((unsigned char *)opaque + ring_size - *masked_cons, buf,    
> >    \
> > +                size - (ring_size - *masked_cons));                        
> >    \
> > +    }                                                                      
> >    \
> > +    *masked_cons = name##_mask(*masked_cons + size, ring_size);            
> >    \
> > +}                                                                          
> >    \
> 
> I like these macros, they make the code that uses them very nice, look
> at patch #2 for example. So far, I tested them by importing them in
> Linux and QEMU, I didn't realize that we have an -ansi check on the
> public headers in Xen (see xen/include/Makefile:headers.chk).
> 
> Because of the static inline functions, there is no hope to compile them
> with -ansi. As soon as we introduce the first user (9pfs, patch #2 of
> this series), the compilation will break.
> 
> At the same time I am very keen on the static inlines and wouldn't want
> to lose them.
> 
> 
> Question 1: Should I move these useful macros elsewhere? If so, where?
> Maybe I could move them to the spec, for example
> docs/misc/9pfs.markdown. Xen doesn't really need them, it's just the
> frontend and backend implementations that could benefit from them.
> 
> If we decide to keep them in ring.h, I guess I'll have to change the
> headers.chk check in xen/include/Makefile for the 9pfs and pvcalls
> headers to be -std=c99 (*only* for 9pfs and pvcalls, of course).

Actually, I noticed there is already a way to remove the ansi compliance
check: I just need to add 9pfs and pvcalls to the filter-out list of
PUBLIC_ANSI_HEADERS in xen/include/Makefile. Is that OK for you?

 
> Question 2: In addition to the static inlines problem, the new macros
> also use memcpy, that needs declaring. I could import <strings.h>, but I
> don't think it makes sense in a Xen public header. Instead, would you
> be OK with me adding the following to ring.h?
> 
> #include <stddef.h> /* needed for size_t */
> extern void *memcpy(void *dest, const void *src, size_t s);
> 
> Of course, if we decide to move the new macros somewhere else, this
> problem goes away with them.

I realized that stddef.h is not allowed either. I am not sure what to do
here. If I remove the ansi check, actually these headers won't be
involved in the build, so there won't be any breakages, and all users
will have a memcpy defined. So maybe we could just get away without
defining memcpy? Other suggestions?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel