Re: [PATCH 3/3] xen/include: Include xen/kconfig.h automatically

[Julien Grall <julien.grall@arm.com>]

Hi Jan,

On 16/02/17 11:10, Jan Beulich wrote:
>>>> On 16.02.17 at 12:01, <andrew.cooper3@citrix.com> wrote:
>> On 16/02/17 10:48, Jan Beulich wrote:
>>>>>> On 16.02.17 at 11:40, <andrew.cooper3@citrix.com> wrote:
>>>> On 16/02/17 10:27, Jan Beulich wrote:
>>>>>>>> On 15.02.17 at 19:10, <andrew.cooper3@citrix.com> wrote:
>>>>>> generated/autoconf.h is already included automatically so CONFIG_* defines
>> are
>>>>>> avaialble.  However, the companion macros such as IS_ENABLED() are not
>>>>>> included.
>>>>>>
>>>>>> Include them uniformally everywhere.
>>>>> Well, if you really think this is a good idea, I'm not going to stand in
>>>>> the way, but why do we need this included everywhere? Many files
>>>>> don't even care about any CONFIG_*, and hence even less so about
>>>>> kconfig.h.
>>>> I am sorry, but you are complaining if I include it unilaterally, and
>>>> also complaining if I include kconfig.h in the specific location where I
>>>> need it.  These are mutually exclusive.
>>> I don't understand - when did I complain about its inclusion where
>>> it's needed? Iirc my complaint was about you adding the inclusion
>>> to */config.h without that header actually using the macros. My
>>> point really is that ideally each C file would get as little cruft as
>>> possible, while at present quite a number of header are being
>>> included by virtually every source file.
>>
>> Your complaint was specifically about me adding it to paging.h so I
>> could use IS_ENABLED() and not out-of-line a trivial function.
>
> Oh, that one: There my view was the other way around: No need
> to include yet another header in one which already gets included
> everywhere, when the new function could easily be out of line (as
> not being performance critical).
>
>> As for general availably, while I agree in general that we have far too
>> much stuff included by default (I have some plans for that), the
>> contents of kconfig.h is fairly small, and exactly the same category of
>> information as config.h
>>
>> I am looking to push for the use of IS_ENABLED() in preference to #ifdef
>> where possible, to reduce code-rot.
>
> Which makes sense, but won't affect said source files not using any
> CONFIG_* in the first place.

At least on ARM, we need CONFIG_* everywhere as the definitions of types 
and structure will change whether you are compiling for ARM64 or ARM.

I would expect the same for common code.

Cheers,

-- 
Julien Grall

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