Re: [PATCH 1/2] rangeset: add RANGESETF_no_print flag

[Jan Beulich <jbeulich@xxxxxxxx>]

On 23.11.2021 08:49, Oleksandr Andrushchenko wrote:
> 
> 
> On 23.11.21 09:38, Jan Beulich wrote:
>> On 22.11.2021 10:28, Oleksandr Andrushchenko wrote:
>>> --- a/xen/include/xen/rangeset.h
>>> +++ b/xen/include/xen/rangeset.h
>>> @@ -51,6 +51,9 @@ void rangeset_limit(
>>>    /* Pretty-print range limits in hexadecimal. */
>>>   #define _RANGESETF_prettyprint_hex 0
>>>   #define RANGESETF_prettyprint_hex  (1U << _RANGESETF_prettyprint_hex)
>>> +/* Do not print entries marked with this flag. */
>>> +#define _RANGESETF_no_print 1
>> There's no need for this, so I'd like to ask that you add ...
>>
>>> +#define RANGESETF_no_print  (1U << _RANGESETF_no_print)
>> ... just this. In due course we should do away with
>> _RANGESETF_prettyprint_hex as well (unless you want to do so right
>> at this occasion).
> Ok, I can remove "#define _RANGESETF_prettyprint_hex 0" as well
> and have both flags defined as
>   /* Pretty-print range limits in hexadecimal. */
> #define RANGESETF_prettyprint_hex  (1U << 0)
> /* Do not print entries marked with this flag. */
> #define RANGESETF_no_print         (1U << 1)
> 
> Or we can use BIT macro here:
> 
> /* Pretty-print range limits in hexadecimal. */
> #define RANGESETF_prettyprint_hex  BIT(0, U)
> /* Do not print entries marked with this flag. */
> #define RANGESETF_no_print         BIT(1, U)
> 
> What's your preference here?

Clearly the former. But this may not be everybody's view. I'm hesitant
towards further uses at least as long as its generalization, GENMASK(),
isn't ready for easy use. It was some time ago that we had a discussion
about that one, supporting my reservations voiced back at the time when
it was introduced into our code base. Iirc there was more than one
issue with it; the immediately obvious one is that it silently does the
wrong thing when the arguments get specified the wrong way round, when
it really would be relatively easy to support either ordering.

Jan