[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 2/7] xen/x86: merge 2 hvm_event_... functions into 1
On 2/10/2016 7:11 PM, Jan Beulich wrote: On 10.02.16 at 18:04, <czuzu@xxxxxxxxxxxxxxx> wrote:On 2/10/2016 6:18 PM, Jan Beulich wrote:On 10.02.16 at 16:50, <czuzu@xxxxxxxxxxxxxxx> 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. 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 Also, why this bias towards #define vs enums? I usually always prefer the latter, they make the code more readable, e.g. it's clearer what can or cannot be passed to a corresponding function parameter. And from an IDE user's point-of-view, I don't understand why I have to read a comment and search for a list of identifiers rather than just clicking on a type name and getting there in a flash. They also make debugging easier, etc.. :). Corneliu. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |