From mboxrd@z Thu Jan 1 00:00:00 1970 From: Corneliu ZUZU Subject: Re: [PATCH v2 2/7] xen/x86: merge 2 hvm_event_... functions into 1 Date: Wed, 10 Feb 2016 22:50:56 +0200 Message-ID: <56BBA2B0.709@bitdefender.com> References: <1455119259-2161-1-git-send-email-czuzu@bitdefender.com> <1455119433-2308-1-git-send-email-czuzu@bitdefender.com> <56BB710302000078000D0B05@prv-mh.provo.novell.com> <56BB6D92.3050300@bitdefender.com> <56BB7D4302000078000D0C22@prv-mh.provo.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56BB7D4302000078000D0C22@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Kevin Tian , Tamas K Lengyel , Keir Fraser , Razvan Cojocaru , Andrew Cooper , xen-devel@lists.xen.org, Jun Nakajima List-Id: xen-devel@lists.xenproject.org On 2/10/2016 7:11 PM, Jan Beulich wrote: >>>> On 10.02.16 at 18:04, wrote: >> On 2/10/2016 6:18 PM, Jan Beulich wrote: >>>>>> On 10.02.16 at 16:50, 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.