Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] 390x/cpumodel: document S390FeatDef.bit not applicable

[Halil Pasic]

On 02/20/2018 05:08 PM, David Hildenbrand wrote:
> On 20.02.2018 17:07, Cornelia Huck wrote:
>> On Tue, 20 Feb 2018 17:04:19 +0100
>> Christian Borntraeger <address@hidden> wrote:
>>
>>> On 02/20/2018 04:55 PM, David Hildenbrand wrote:
>>>> On 20.02.2018 16:53, Cornelia Huck wrote:  
>>>>> On Tue, 20 Feb 2018 16:07:13 +0100
>>>>> Halil Pasic <address@hidden> wrote:
>>>>>  
>>>>>> The 'bit' field of the 'S390FeatDef' structure is not applicable to all
>>>>>> it's instances. Currently a this field is not applicable, and remains  
>>>>>
>>>>> s/it's/its/
>>>>>
>>>>> s/a this/this/
>>>>>  
>>>>>> unused, iff the feature is of type S390_FEAT_TYPE_MISC. Having the value >>>>>> 0
>>>>>> specified for multiple such feature definition  was a little confusing,
>>>>>> as it's a perfectly legit bit value, and as usually the value of the bit
>>>>>> field is ought to be unique for each feature.
>>>>>>
>>>>>> Let's document this, and hopefully reduce the potential for confusion.
>>>>>>
>>>>>> Signed-off-by: Halil Pasic <address@hidden>
>>>>>> ---
>>>>>>
>>>>>> Hi!
>>>>>>
>>>>>> This may be an overkill. A comment where the misc features
>>>>>> are defined would do to, but I think this is nicer. So
>>>>>> I decided to try it with this approach first.  
>>>>>
>>>>> Is there likely to be anything else than FEAT_MISC _not_ using .bit? If
>>>>> not, would it be better to at a comment to the FEAT_MISC definition?  
>>>>
>>>> Doubt it right now. I would sign the "overkill" part :)  
>>>
>>> I can cconfirm that this code caused some questions and it took me some
>>> minutes to remember why 0 and 0 was ok. So I certainly want to have a 
>>> comment
>>> of some form.
>>>
>>
>> I'd prefer a comment about FEAT_MISC usage rather than a magic value.

As I said elsewhere such stuff used to be called 'extremal element' at
the university. We used the term 'magic value' for hard coded constants.
But the jargon can be different elsewhere.

>>
> 
> We can also add FEAT_INIT_MISC. And add a comment in the initializer.

I don't care that much about aesthetics. FEAT_INT_MISC would have the
benefit of making the definitions shorter. But whatever you decide on.