Re: [Xen-devel] [PATCH v1 Altp2m cleanup 1/3] altp2m cleanup work

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.06.16 at 20:23, <paul.c.lai@xxxxxxxxx> wrote:

First of all - please don't top post.

> I'm opposed to moving HVMOP_cmd_min and HVMOP_cmd_max somewhere else.  That 
> would make reading/understanding of the macros more difficult.  This practice 
> is common.  Also, If min & max are defined elsewhere, it will be more likely 
> to lead to mistakes/bugs. 

I understand that, but I'm going to nak any patch that introduces
clutter (which then will need to be retained forever) to the public
interface. A possible compromise might be to frame these in __XEN__
conditionals, but this would need to be outweighed by the resulting
benefits, which I'm not convinced of.

> The use of "_min" and "_max" should be quite clear and is common use in 
> linux code; Yes, I know this is xen code and I see it here too.  If there's a 
> better way, please propose the better.  Maybe you're suggesting the macro 
> names should be all caps: 
>   HVMOP_CMD_MIN, HVMOP_CMD_MAX
> ?  I was following the coding style within the file itself.

The problem isn't the use of min and max, but the selected names
suggesting these are the minimum/maximum HVMOPs, whereas
- afaict - you really mean to frame the altp2m subset. Which btw,
together with the above, points at another issue here: What if
later another altp2m subop gets added, and especially when its
new value ends up not being adjacent to the existing range?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel