From: George Dunlap <george.dunlap@citrix.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
Wei Liu <wei.liu2@citrix.com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
George Dunlap <George.Dunlap@eu.citrix.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
IanJackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
Julien Grall <julien.grall@arm.com>,
xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] [PATCH v2] CODING_STYLE: document intended usage of types
Date: Mon, 5 Aug 2019 15:35:59 +0100 [thread overview]
Message-ID: <0a8031c0-b668-eeb1-a9a2-659b52aaf98d@citrix.com> (raw)
In-Reply-To: <72580391-d34e-aaf9-2e41-ab1df5967408@suse.com>
On 8/5/19 2:19 PM, Jan Beulich wrote:
> 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
OK, well this gives me an idea what you mean. I'm OK with this; but it
would probably be good if one of the ARM guys Acked it as well.
Acked-by: George Dunlap <george.dunlap@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
prev parent reply other threads:[~2019-08-05 14:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-26 9:31 [PATCH v2] CODING_STYLE: document intended usage of types Jan Beulich
2019-08-05 10:58 ` [Xen-devel] " George Dunlap
2019-08-05 11:55 ` Jan Beulich
2019-08-05 13:01 ` George Dunlap
2019-08-05 13:19 ` Jan Beulich
2019-08-05 14:35 ` George Dunlap [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0a8031c0-b668-eeb1-a9a2-659b52aaf98d@citrix.com \
--to=george.dunlap@citrix.com \
--cc=George.Dunlap@eu.citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=JBeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=julien.grall@arm.com \
--cc=konrad.wilk@oracle.com \
--cc=sstabellini@kernel.org \
--cc=tim@xen.org \
--cc=wei.liu2@citrix.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).