[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v10 2/6] xen: introduce DEFINE_SYMBOL



On Tue, 26 Feb 2019, Jan Beulich wrote:
> >>> On 25.02.19 at 21:50, <sstabellini@xxxxxxxxxx> wrote:
> > --- a/xen/include/xen/compiler.h
> > +++ b/xen/include/xen/compiler.h
> > @@ -99,6 +99,38 @@
> >      __asm__ ("" : "=r"(__ptr) : "0"(ptr));      \
> >      (typeof(ptr)) (__ptr + (off)); })
> >  
> > +
> > +/*
> > + * Declare start and end array variables in C corresponding to existing
> > + * linker symbols.
> 
> You validly say "declare" here, so why ...
> 
> > + * Two static inline functions are declared to do comparisons and
> > + * subtractions between these variables.
> > + *
> > + * The end variable is declared with a different type to make sure that
> > + * the static inline functions cannot be misused.
> > + */
> > +#define DEFINE_SYMBOL(type, name, start_name, end_name)                    
> >    \
> 
> ... do you use DEFINE here?
> 
> How about DECLARE_ARRAY_BOUNDS(tag, name) using
> tag ## _t as type, tag ## _lt etc as function names, and
> name ## _start / name ## _end as start / end symbols. To
> accommodate things like _etext, the above could in fact expand
> to DECLARE_BOUNDS(tag, name ## _start, name ## _end)
> allowing this second macro then to also be used like
> DECLARE_BOUNDS(text, _stext, _etext).

I am fine with renaming the macro. It is also fine to provide a wrapper
macro which automatically sets the most common variable names.

However, in your example both DECLARE_BOUNDS and DECLARE_ARRAY_BOUNDS
end up assuming that the type is tag ## _t. Currently many of our
variable types don't follow this naming pattern (struct alt_instr,
struct device_desc, struct acpi_device_desc, just to name the first
three I found). I don't think it is a good idea to introduce even more
renaming as part of this series. I suggest to do this instead:

#define DECLARE_ARRAY_BOUNDS(type, name) DECLARE_BOUNDS(type, name, name ## 
_start, name ## _end)

Where type is the type, name is used for abstract_ ## name, name ## _lt
and name ## _diff.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.