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

[David Hildenbrand <david@redhat.com>]

On 20.02.2018 17:07, Cornelia Huck wrote:
> On Tue, 20 Feb 2018 17:04:19 +0100
> Christian Borntraeger <borntraeger@de.ibm.com> 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 <pasic@linux.vnet.ibm.com> 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 <pasic@linux.vnet.ibm.com>
>>>>> ---
>>>>>
>>>>> 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.
> 

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

-- 

Thanks,

David / dhildenb