[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 0/6] xen/arm: Move in/out code to/from init section

>>> On 02.02.15 at 13:52, <julien.grall@xxxxxxxxxx> wrote:
> On 02/02/15 11:15, Jan Beulich wrote:
>>>>> On 02.02.15 at 11:58, <Ian.Campbell@xxxxxxxxxx> wrote:
>>> On Fri, 2015-01-30 at 11:33 +0000, Julien Grall wrote:
>>>> On 30/01/15 11:30, Ian Campbell wrote:
>>>>> On Thu, 2015-01-29 at 18:32 +0000, Julien Grall wrote:
>>>>>> Hello,
>>>>>> Ping? Any more review for this version of this series?
>>>>> I was awaiting a version with the declarations in the right places as
>>>>> pointed out by Andy (including his point about definition vs. prototype
>>>>> I'm afraid, which is the style we use).
>>>> I'm not convince about your last point. We have many places (if not all
>>>> on ARM) where __init is used in the header.
>>> Those are mistakes, it seems.
>>>> Some of them was even added by you ;). So I would prefer if we keep
>>>> them, it's more readable. FWIW Linux does it too.
>>> I personally don't particularly care where these things go, but other
>>> hypervisor maintainers seem to and we should aim for the code base to be
>>> consistent where possible.
>>> If we want to change this coding style then we should do that directly
>>> and explicitly, not through the backdoor by just ignoring the policy.
>> And I think we should strive to keep pointless redundancy down:
>> There's no point in repeating attributes - either they belong on the
>> declaration (when producer and consumers care), or they belong
>> on the definition (when only the producer cares). Repeating them
>> on the definition when the declaration already has them only results
>> in needless churn when such an attribute later gets changed/
>> removed, and having producer-relevant attributes on declarations
>> means needless extra work by the compiler (however little that may
>> be - it adds up).
> I think we should allow the annotation in both place. When a developer
> is looking for the prototype of the function, it will likely look at the
> headers and not the implementation itself.
> This may lead to call an init function in code running after the boot. I
> don't think we can catch it easily during review.

This example in particular makes clear that looking at just the
declaration is insufficient in certain cases. Whoever is looking
for specific properties of a functions needs to know which ones
(s)he cares about, and then look in the appropriate place. Unless
a rule gets put in place overriding my personal opinion on this, I'm
not going to ack or otherwise accept needless code duplication.
If the ARM maintainers feel differently, so be it.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.