Re: [Xen-devel] [PATCH v9 2/5] xen: introduce SYMBOLS_SUBTRACT and SYMBOLS_COMPARE

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.02.19 at 13:01, <ian.jackson@xxxxxxxxxx> wrote:
> I would particularly welcome the opinion of hypervisor maintainers on
> my type safety point, below.

I agree with the requirements you put forward; I think I'd
prefer the inline function versions I had suggested (or
something similar) over macros though, not the least because
they come with "built-in" type safety, rather than grafted one
(by adding "pseudo" comparisons).

> Stefano Stabellini writes ("[Xen-devel] [PATCH v9 2/5] xen: introduce 
> SYMBOLS_SUBTRACT and SYMBOLS_COMPARE"):
>> +/*
>> + * Calculate (end - start), where start and/or end are linker symbols,
>> + * returning a ptrdiff_t. The size is in units of start's referent.
>> + */
>> +#define SYMBOLS_SUBTRACT(end, start) ({                                     
>>   \
>> +    (ptrdiff_t)((uintptr_t)(end) - (uintptr_t)(start)) / sizeof(*start);    
>>   \
>> +})
> 
> I'm afraid I have several problems with the details of these macros.
> 
> Firstly, I really don't like the lack a type check, which was present
> in my proposal.  For example, given:
>      extern struct wombat _foos_start[];
>      extern char _end[];
> and your macro definition, the two expressions
>       SYMBOLS_SUBTRACT(_foos_start, _end)
>      -SYMBOLS_SUBTRACT(_end, _foos_start)
> produce different values because one is divided by sizeof(struct
> wombat) and the other by sizeof(char).  This is hardly desirable.
> 
> 
> Secondly, I find the argument ordering extremely confusing.  With your
> macros DIFFERENCE(start,end) is negative but COMPARE(start,end) is
> positive.  I suggest that you call the macro DIFFERENCE and have
> DIFFERENCE(start,end) be positive.

Indeed having to put end before start in either macro invocation
is prone to be got wrong. In the common case this will be noticed
quickly, but even then it's likely one extra compile and test run to
notice that there's something wrong.

However, I realize this is to keep use sites look more like
"end - start", which has its merits as well.

> Thirdly, in an earlier exchange:
> [...]

And fourth, looking at just what's left in context, I see that the
macro argument uses are incompletely parenthesized.

Furthermore - do we really need both a subtract and a compare
construct? The result subtract produces can be used for
comparison purposes as well, after all (just like all CPUs I know
details of implement [integer] compare as a subtract discarding
its numeric result, instead [or only] updating certain status flags).

Jan



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