Re: [Xen-devel] [v7][RFC][PATCH 04/13] hvmloader/util: get reserved device memory maps

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.10.14 at 03:20, <tiejun.chen@xxxxxxxxx> wrote:
> On 2014/10/30 17:13, Jan Beulich wrote:
>>>>> On 30.10.14 at 06:55, <tiejun.chen@xxxxxxxxx> wrote:
>>> On 2014/10/29 17:05, Jan Beulich wrote:
>>>>>>> On 29.10.14 at 07:54, <tiejun.chen@xxxxxxxxx> wrote:
>>>>> Looks I can remove those stuff from util.h and just add 'extern' to them
>>>>> when we really need them.
>>>>
>>>> Please stop thinking this way. Declarations for things defined in .c
>>>> files are to be present in headers, and the defining .c file has to
>>>> include that header (making sure declaration and definition are and
>>>> remain in sync). I hate having to again repeat my remark that you
>>>> shouldn't forget it's not application code that you're modifying.
>>>> Robust and maintainable code are a requirement in the hypervisor
>>>> (and, as said it being an extension of it, hvmloader). Which - just
>>>> to avoid any misunderstanding - isn't to say that this shouldn't also
>>>> apply to application code. It's just that in the hypervisor and kernel
>>>> (and certain other code system components) the consequences of
>>>> being lax are much more severe.
>>>
>>> Okay. But currently, the pci.c file already include 'util.h' and
>>> '<xen/memory.h>,
>>>
>>> #include "util.h"
>>> ...
>>> #include <xen/memory.h>
>>>
>>> We can't redefine struct xen_reserved_device_memory in util.h.
>>
>> Redefine? I said forward declare.
> 
> Seems we just need to declare hvm_get_reserved_device_memory_map() in 
> the head file, tools/firmware/hvmloader/util.h,
> 
> unsigned int hvm_get_reserved_device_memory_map(void);

To me this looks very much like poor programming style, even if in
the context of hvmloader communicating information via global
variables rather than function arguments and return values is
generally possible.

Jan


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