Re: [PATCH v2] bitops: Fix incorrect value in comment

[Julien Grall <julien@xxxxxxx>]

Hi,

On 01/12/2021 09:38, Jan Beulich wrote:
> On 01.12.2021 10:33, Julien Grall wrote:
> > On 30/11/2021 18:12, Ayan Kumar Halder wrote:
> > > --- a/xen/include/xen/bitops.h
> > > +++ b/xen/include/xen/bitops.h
> > > @@ -5,7 +5,7 @@
> > >    /*
> > >     * Create a contiguous bitmask starting at bit position @l and ending at
> > >     * position @h. For example
> > > - * GENMASK(30, 21) gives us the 32bit vector 0x01fe00000.
> > > + * GENMASK(30, 21) gives us the 32bit value 0x7fe00000.
> > ... there are two extra changes here:
> >     1) The bitmask is now described with 8-characters (rather than 9)
> >     2) 'vector' is replaced with 'value'
> >
> > The former makes sense to me, but it is not clear to me why the latter
> > should be changed.
> Would you mind explaining to me in which way you see "vector" accurately
> describe the entity talked about?
This can be seen as a vector of bit. I can see why people may think 
otherwise. However... if you think it doesn't describe it accurately, 
then I think this ought to be changed in Linux first (where the code and 
comment comes from).
> I also think the commit message is quite fine as is.
IMHO, this is similar to when one do coding style change in a patch. 
They are unrelated but would be acceptable so long they are explained in 
the commit message.
What I request is something like:

"GENMASK(30, 21) should be 0x7fe00000 and only use 8-characters (it is a 
32-bit comment). Fixed this in the comment.
Take the opportunity to replace 'vector' with 'value' because..."

This is simple enough and clarify what is the intent of the patch.

Cheers,

--
Julien Grall