Re: [PATCH v2 2/7] xen/x86: merge 2 hvm_event_... functions into 1

[Corneliu ZUZU <czuzu@bitdefender.com>]

On 2/10/2016 7:11 PM, Jan Beulich wrote:
>>>> On 10.02.16 at 18:04, <czuzu@bitdefender.com> wrote:
>> On 2/10/2016 6:18 PM, Jan Beulich wrote:
>>>>>> On 10.02.16 at 16:50, <czuzu@bitdefender.com> wrote:
>>>> --- a/xen/include/asm-x86/hvm/event.h
>>>> +++ b/xen/include/asm-x86/hvm/event.h
>>>> @@ -17,6 +17,12 @@
>>>>    #ifndef __ASM_X86_HVM_EVENT_H__
>>>>    #define __ASM_X86_HVM_EVENT_H__
>>>>    
>>>> +enum hvm_event_breakpoint_type
>>>> +{
>>>> +    HVM_EVENT_SOFTWARE_BREAKPOINT,
>>>> +    HVM_EVENT_SINGLESTEP_BREAKPOINT,
>>>> +};
>>> I don't see what good it does to put existing constants into an
>>> enum.
>> As Andrew pointed out, an enum was requested in v1 instead of the
>> single_step param.
>> One could use the already existing VM_EVENT_REASON_* constants, but
>> conceptually this
>> function only involves a subset of those (i.e. *breakpoint vm-events*).
> Re-using existing constants would seem fine to me.

Yes but then IMHO conceptually that would be wrong, since it would be 
like saying
"that param can be any type of vm-event", even though it can actually be 
only from a
subset of vm-event types and it will never be *any* type of vm-event.

> I only now realize that I've made a mistake while looking at the
> above - the capitals made it implicitly "obvious" to me that they're
> on the right side of an assignment. Please use capitals only for
> #define-d constants, not enumerated ones.
>
> Jan

Most examples of existing enums I've looked at were capital-case, I thought
they're supposed to be like that. Could you please provide an example on how
enum values should look? (there isn't any in CODING_STYLE)

Let me know if you still want me to change this and how.

Corneliu.