Re: [Xen-devel] [PATCH 2/2] dom_cow is needed for mem-sharing only

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.05.19 at 12:10, <julien.grall@xxxxxxx> wrote:
> On 5/31/19 11:03 AM, Jan Beulich wrote:
>>>>> On 31.05.19 at 11:52, <julien.grall@xxxxxxx> wrote:
>>> On 5/31/19 10:49 AM, Jan Beulich wrote:
>>>>>>> On 31.05.19 at 11:42, <julien.grall@xxxxxxx> wrote:
>>>>> On 5/31/19 10:35 AM, Jan Beulich wrote:
>>>>>> --- a/xen/include/xen/mm.h
>>>>>> +++ b/xen/include/xen/mm.h
>>>>>> @@ -644,6 +644,9 @@ static inline void filtered_flush_tlb_ma
>>>>>>     
>>>>>>     /* Private domain structs for DOMID_XEN, DOMID_IO, etc. */
>>>>>>     extern struct domain *dom_xen, *dom_io, *dom_cow;
>>>>>
>>>>> Don't you want to protect dom_cow with "#ifdef CONFIG_HAS_MEM_SHARING"?
>>>>
>>>> There's no need to with ...
>>>>
>>>>>> +#ifndef CONFIG_HAS_MEM_SHARING
>>>>>> +# define dom_cow NULL
>>>>>> +#endif
>>>>
>>>> ... this, and this way there's less clutter overall.
>>>
>>> I am all for avoiding cluttering but not at the expense of making the
>>> code less intuitive. In this case, I would prefer the decleration
>>> dom_cow to be guarded.
>> 
>> While it would be easy enough to do, I fail to see how the chosen
>> construct is not "intuitive".
> 
> Even if I know the pre-preprocessor will do the right thing here, you 
> could spare the developper to trip over this code and wonder why it is 
> first defined and then override with NULL.

To be honest, an unconditional declaration with a conditional
override doesn't leave much to wonder about. I'll wait to see
what other maintainers think before deciding either way.

>> In fact I don't think this would be the
>> first instance of a #define overriding a prior declaration. Doing so
>> utilizes one of the very basic C preprocessor principles.
> 
> You are the first one usually to say that some choices in Xen were not 
> correct and therefore no more instance should be added.

Oh, did my earlier reply sound as if I'm not happy about those
mentioned instances? I certainly didn't mean it to be that way -
some of them I'm liable to have introduced myself, and I
continue to approve of them being there.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel