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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/02/17 11:10, Jan Beulich wrote:
>>>> On 16.02.17 at 12:01, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 16/02/17 10:48, Jan Beulich wrote:
>>>>>> On 16.02.17 at 11:40, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> On 16/02/17 10:27, Jan Beulich wrote:
>>>>>>>> On 15.02.17 at 19:10, <andrew.cooper3@xxxxxxxxxx> 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.

We already include CONFIG_* everywhere.  All this change does is
consistently add IS_ENABLED() alongside, so it can be used when CONFIG_*
are available.

If we have occasion in the future to reconsider having the CONFIG_*
variables unilaterally included, then fine, but the current state of the
code is the worst of all options.

~Andrew

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