qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 2/5] HACKING: add C type rules


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH 2/5] HACKING: add C type rules
Date: Sun, 15 Aug 2010 16:37:04 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.11) Gecko/20100713 Lightning/1.0b1 Thunderbird/3.0.6

Hi Blue,

Thanks for putting this document together.  It should be quiet helpful!

On 08/15/2010 12:50 PM, Blue Swirl wrote:
Add C type rules, adapted from libvirt HACKING. Also include
a description of special QEMU scalar types.

Move typedef rule from CODING_STYLE rule 3 to HACKING rule 6
where it belongs.

Signed-off-by: Blue Swirl<address@hidden>
---
  CODING_STYLE |    3 --
  HACKING      |   64 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  2 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/CODING_STYLE b/CODING_STYLE
index 92036f3..2c8268d 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -46,9 +46,6 @@ names are
lower_case_with_underscores_ending_with_a_t, like the POSIX
  uint64_t and family.  Note that this last convention contradicts POSIX
  and is therefore likely to be changed.

-Typedefs are used to eliminate the redundant 'struct' keyword.  It is the
-QEMU coding style.
-
  When wrapping standard library functions, use the prefix qemu_ to alert
  readers that they are seeing a wrapped version; otherwise avoid this prefix.

diff --git a/HACKING b/HACKING
index e60aa48..7c6b49e 100644
--- a/HACKING
+++ b/HACKING
@@ -4,3 +4,67 @@ For variadic macros, stick with C99 syntax:

  #define DPRINTF(fmt, ...)                                       \
      do { printf("IRQ: " fmt, ## __VA_ARGS__); } while (0)
+
+2. C types
+
+It should be common sense to use the right type, but we have collected
+a few useful guidelines here.
+
+2.1. Scalars
+
+If you're using "int" or "long", odds are good that there's a better type.
+If a variable is counting something, it should be declared with an
+unsigned type.
+
+If it's host memory-size related, size_t should be a good choice (use
+ssize_t only if required). Guest RAM memory offsets must use ram_addr_t,
+but only for RAM, it may not cover whole guest address space.

This needs a little more explanation. There are two different address spaces. target_phys_addr_t represents guest RAM physical addresses. ram_addr_t is a QEMU internal address space that maps guest RAM physical addresses into an intermediate address space that can map to host virtual address spaces.

Generally speaking, the size of guest memory can always fit into ram_addr_t but it would not be correct to store an actual guest physical address in a ram_addr_t.

+If it's file-size related, use off_t.
+If it's file-offset related (i.e., signed), use off_t.
+If it's just counting small numbers use "unsigned int";
+(on all but oddball embedded systems, you can assume that that
+type is at least four bytes wide).
+
+In the event that you require a specific width, use a standard type
+like int32_t, uint32_t, uint64_t, etc.  The specific types are
+mandatory for VMState fields.
+
+Don't use Linux kernel internal types like u32, __u32 or __le32.
+
+Use target_phys_addr_t for hardware physical addresses except pcibus_t
+for PCI addresses.  Use target_ulong (or abi_ulong) for CPU
+virtual addresses, however devices should not need to use target_ulong.
+
+While using "bool" is good for readability, it comes with minor caveats:
+ - Don't use "bool" in places where the type size must be constant across
+   all systems, like public interfaces and on-the-wire protocols.
+ - Don't compare a bool variable against the literal, "true",
+   since a value with a logical non-false value need not be "1".
+   I.e., don't write "if (seen == true) ...".  Rather, write "if (seen)...".
+
+Of course, take all of the above with a grain of salt.  If you're about
+to use some system interface that requires a type like size_t, pid_t or
+off_t, use matching types for any corresponding variables.
+
+Also, if you try to use e.g., "unsigned int" as a type, and that
+conflicts with the signedness of a related variable, sometimes
+it's best just to use the *wrong* type, if "pulling the thread"
+and fixing all related variables would be too invasive.
+
+Finally, while using descriptive types is important, be careful not to
+go overboard.  If whatever you're doing causes warnings, or requires
+casts, then reconsider or ask for help.
+
+2.2. Pointers
+
+Ensure that all of your pointers are "const-correct".
+Unless a pointer is used to modify the pointed-to storage,
+give it the "const" attribute.  That way, the reader knows
+up-front that this is a read-only pointer.  Perhaps more
+importantly, if we're diligent about this, when you see a non-const
+pointer, you're guaranteed that it is used to modify the storage
+it points to, or it is aliased to another pointer that is.
+
+2.3. Typedefs
+Typedefs are used to eliminate the redundant 'struct' keyword.

It's worth mention the reserved namespaces in C. That is, underscore capital, double underscore, and underscore 't' suffixes should be avoid.

Regards,

Anthony Liguori



reply via email to

[Prev in Thread] Current Thread [Next in Thread]