On Sun, Aug 15, 2010 at 9:37 PM, Anthony Liguori <anth...@codemonkey.ws> wrote: > 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<blauwir...@gmail.com> >> --- >> 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.
Not only RAM, it can be MMIO. > 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. I'll add this to the next version. > >> +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. OK, thanks for the review.