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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 27 Mar 2017, Jan Beulich wrote:
> >>> On 24.03.17 at 19:31, <sstabellini@xxxxxxxxxx> wrote:
> > --- a/xen/include/public/io/ring.h
> > +++ b/xen/include/public/io/ring.h
> > @@ -27,7 +27,18 @@
> >  #ifndef __XEN_PUBLIC_IO_RING_H__
> >  #define __XEN_PUBLIC_IO_RING_H__
> >  
> > +/* 
> > + * When #include'ing this header, you need to provide the following
> > + * declarations upfront:
> > + * - standard integers types (uint8_t, uint16_t, etc)
> > + * - size_t
> > + * - memcpy
> > + * These declarations are provided by stdint.h and string.h of the
> > + * standard headers.
> > + */
> > +
> >  #include "../xen-compat.h"
> > +#include "../grant_table.h"
> 
> I'd prefer this to be added to the prereqs, as the header itself will
> - afaict - compile fine without the #include above.

It does not, see "grant_ref_t ref[]" in struct name##_data_intf.


> Also the list of
> prereqs only applies to people who mean to use the new macros,
> doesn't it? In that case the comment should say so.

Standard integers types are needed also for the old macros. I'll
clarify what's needed for what.


> > +#ifndef PAGE_SHIFT
> > +#define PAGE_SHIFT 12
> > +#endif
> 
> ??? (I guess this should be another prereq?)

Yes, the PAGE_SHIFT definition could be a prereq, but I thought that
these 3 lines are small enough that they would be harmless. I am happy
to make it a prereq though, I'll remove it.


> > +#define XEN_FLEX_RING_SIZE(order)                                          
> >    \
> > +    (1UL << (order + PAGE_SHIFT - 1))
> 
> Please parenthesize uses of macro parameters.

OK


> > +static inline unsigned char* name##_get_ring_ptr(unsigned char *buf,       
> >    \
> 
> Misordered * and space (return type).

I'll fix.

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