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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 28 Mar 2017, Jan Beulich wrote:
> >>> Stefano Stabellini <sstabellini@xxxxxxxxxx> 03/27/17 10:53 PM >>>
> >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.
> 
> The use is in a macro _definition_, and iirc the macro is not being _used_.
> (Almost) any garbage can be put in unused macro definitions without causing
> compile breakage.

I think I understand: you prefer to remove the inclusion from ring.h,
add it to the prereqs, and add the #include to 9pfs.h and pvcalls.h.
I'll do that.


> >> > +#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.
> 
> The problem isn't size or anything, but the risk to clash with a _later_ 
> definition
> (due to a subsequent #include). The name, after all, isn't in the XEN_ name 
> space.
 
Yes, you are right, and as I replied afterward we cannot rely on the
PAGE_SHIFT set by the system anyway. If that's OK I'll define
XEN_PAGE_SHIFT instead.

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