Re: [Xen-devel] [PATCH v2] CODING_STYLE: document intended usage of types

[Jan Beulich <JBeulich@xxxxxxxx>]

On 05.08.2019 15:01, George Dunlap wrote:
> On 8/5/19 12:55 PM, Jan Beulich wrote:
>> On 05.08.2019 12:58, George Dunlap wrote:
>>> On 11/26/18 9:31 AM, Jan Beulich wrote:
>>>> +Fixed width types should only be used when a fixed width quantity is
>>>> +meant (which for example may be a value read from or to be written to a
>>>> +register).
>>>
>>> I'm having trouble understanding the intent / implications of this one.
>>>    Can you give me an example of where you've seen a fixed width type used
>>> inappropriately?
>>
>> Grep the code base for "uint32_t size" for example. These should
>> (almost?) all be unsigned int (or, where necessary, size_t).
> 
> Inside the hypervisor codebase anyway, I see these patterns for
> `uint32_t size`:
> 
> 1. Inside tracing structures, where the code may be used either by
> 32-bit or 64-bit userspace tools

I simply assume in these cases use of fixed width types is
intended.

> 2. Inside headers for public interfaces (same reason).

Here fixed width types are definitely the right choice.

> 3. In function signatures for emulation code.  I assume this is because
> sizes are architecturally defined.

Taking null_read() as an example - no, there's no need for a fixed
width type here. Even if the value was read from a register,
propagating the value still only needs to guarantee no truncation.
But the value can't come from a register directly anyway, or else
the type would need to be uint64_t. The type "addr" is wrongly
using uint64_t here, too, in my opinion: It should be unsigned long
or paddr_t, depending on whether we're talking of linear or physical
addresses (I think it's the latter here).

> 4. Inside decompression code, to interface with public sizes.

I don't think there's any interfacing with "public" structures
there.

5. sysctl_cov_op() should again use unsigned int
6. struct elf_sym_header too should use unsigned int
7. struct hvm_domain_context may want to continue to use uint8_t
    (albeit unsigned char would be quite fine as well), but its
    two uint32_t uses could once again be unsigned int
etc

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel