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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.02.17 at 12:59, <andrew.cooper3@xxxxxxxxxx> wrote:
> 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.

The relevant aspect isn't CONFIG_* being available, but any of
them being actually used.

> 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.

I don't think so, but as said, I'm not meaning to stand in the way of
this patch going in (as making the current situation only marginally
worse).

Jan


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