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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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).


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.

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