headers.chk failing under certain circumstances.

headers.chk failing under certain circumstances.

[Andrew Cooper]

Hello,

Back in changeset 19772:aaab04808ee7, you introduced headers.chk to
check the header files for ansi conformance.

To fix the current 32/64bit interaction errors with the kexec
hypercalls, I need to use uint64_aligned_t as a datatype.  For the
normal compile, this is all fine, but as header.chk does not define
__XEN__ or __XEN_TOOLS__, the declaration of uint64_aligned_t is never
made, leading to the check failing.

There are other hypercall interfaces which use these datatypes: domctl,
sysctl and hvm_op, but these header files are explicitly filtered out
from the prerequisites for header.chk.  Given that uint64_aligned_t is a
sensible datatype to be using with the hypercall interface, fixing the
check seems to be the correct solution.

In your oppinion, which is the best course of action? To define __XEN__
or __XEN_TOOLS__ as part of the check (this throws up other errors as
part of the check process, suggesting that the header files are hiding
ansi non-conformance in certain blocks), or dont predicate the
definition of uint64_aligned_t on the presence of the above defines?

In addition, why are certain header files excluded from being checked? 
Does this imply that then should be fixed up to be ansi conformant as well?

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

Re: headers.chk failing under certain circumstances.

[Jan Beulich]

>>> On 13.12.11 at 14:38, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> To fix the current 32/64bit interaction errors with the kexec
> hypercalls, I need to use uint64_aligned_t as a datatype.  For the
> normal compile, this is all fine, but as header.chk does not define
> __XEN__ or __XEN_TOOLS__, the declaration of uint64_aligned_t is never
> made, leading to the check failing.
> 
> There are other hypercall interfaces which use these datatypes: domctl,
> sysctl and hvm_op, but these header files are explicitly filtered out
> from the prerequisites for header.chk.

This is really intentional for the domctl and sysctl cases - these are
"internal" interfaces (between the tools and the hypervisor), and
hence can be more relaxed in style.

Pretty much the same goes for the hvm op definitions - we simply
assume that users of this use a compiler capable of dealing with this
(I'm not fully convinced this is The Right Thing To Do, but it's the
status quo at least).

> Given that uint64_aligned_t is a
> sensible datatype to be using with the hypercall interface, fixing the
> check seems to be the correct solution.

Sensible or not, you would have to find a way to define such a type
in a compiler-independent (i.e. ANSI conforming) manner, and I'm
afraid you won't be able to.

> In your oppinion, which is the best course of action? To define __XEN__
> or __XEN_TOOLS__ as part of the check (this throws up other errors as
> part of the check process, suggesting that the header files are hiding
> ansi non-conformance in certain blocks), or dont predicate the
> definition of uint64_aligned_t on the presence of the above defines?

__XEN__ and __XEN_TOOLS__ must specifically not be defined for
these checks (and these symbols indeed get used to frame certain
definitions that are considered "internal" as outlined above - obviously,
by framing any definition this way we hide them from "foreign"
consumers *and* the [pointless in this case] style checking).

So bottom line is - no, you can't use uint64_aligned_t as much as any
other non-ANSI extension. Preventing anyone to try is what the
check was introduced for.

Jan

Re: headers.chk failing under certain circumstances.

[Keir Fraser]

On 13/12/2011 14:17, "Jan Beulich" <JBeulich@suse.com> wrote:

>> Given that uint64_aligned_t is a
>> sensible datatype to be using with the hypercall interface, fixing the
>> check seems to be the correct solution.
> 
> Sensible or not, you would have to find a way to define such a type
> in a compiler-independent (i.e. ANSI conforming) manner, and I'm
> afraid you won't be able to.

Yes, as barbaric as it may seem you just have to make sure that uint64_t
gets naturally 8-byte aligned. Luckily none of our interface structures are
particularly large, and I believe you can get them checked for 32/64-bit
invariance by adding them to xlat.lst.

 -- Keir

Re: headers.chk failing under certain circumstances.

[Jan Beulich]

>>> On 13.12.11 at 15:21, Keir Fraser <keir@xen.org> wrote:
> On 13/12/2011 14:17, "Jan Beulich" <JBeulich@suse.com> wrote:
> 
>>> Given that uint64_aligned_t is a
>>> sensible datatype to be using with the hypercall interface, fixing the
>>> check seems to be the correct solution.
>> 
>> Sensible or not, you would have to find a way to define such a type
>> in a compiler-independent (i.e. ANSI conforming) manner, and I'm
>> afraid you won't be able to.
> 
> Yes, as barbaric as it may seem you just have to make sure that uint64_t
> gets naturally 8-byte aligned. Luckily none of our interface structures are
> particularly large, and I believe you can get them checked for 32/64-bit
> invariance by adding them to xlat.lst.

... and invoking the resultant check macro somewhere in the sources.

Jan

Re: headers.chk failing under certain circumstances.

[Andrew Cooper]

On 13/12/11 14:25, Jan Beulich wrote:
>>>> On 13.12.11 at 15:21, Keir Fraser <keir@xen.org> wrote:
>> On 13/12/2011 14:17, "Jan Beulich" <JBeulich@suse.com> wrote:
>>
>>>> Given that uint64_aligned_t is a
>>>> sensible datatype to be using with the hypercall interface, fixing the
>>>> check seems to be the correct solution.
>>> Sensible or not, you would have to find a way to define such a type
>>> in a compiler-independent (i.e. ANSI conforming) manner, and I'm
>>> afraid you won't be able to.
>> Yes, as barbaric as it may seem you just have to make sure that uint64_t
>> gets naturally 8-byte aligned. Luckily none of our interface structures are
>> particularly large, and I believe you can get them checked for 32/64-bit
>> invariance by adding them to xlat.lst.
> ... and invoking the resultant check macro somewhere in the sources.
>
> Jan

Righ ok.  Thanks for the explanation.

Luckily, it appears that all my uint64_t's do actually align on 8 bytes
so I will just use that.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com