Re: [Qemu-devel] [PATCH v3 2/4] docs: merge HACKING.rst contents into CODING_STYLE.rst

2019-09-05 Thread Eric Blake
On 8/29/19 11:50 AM, Daniel P. Berrangé wrote:
> The split of information between the two docs is rather arbitary and
> unclear. It is simpler for contributors if all the information is in
> one file.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  CODING_STYLE.rst | 296 ++
>  HACKING.rst  | 300 ---
>  README.rst   |   2 +-
>  3 files changed, 297 insertions(+), 301 deletions(-)
>  delete mode 100644 HACKING.rst
> 

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v3 2/4] docs: merge HACKING.rst contents into CODING_STYLE.rst

2019-08-29 Thread Daniel P . Berrangé
The split of information between the two docs is rather arbitary and
unclear. It is simpler for contributors if all the information is in
one file.

Signed-off-by: Daniel P. Berrangé 
---
 CODING_STYLE.rst | 296 ++
 HACKING.rst  | 300 ---
 README.rst   |   2 +-
 3 files changed, 297 insertions(+), 301 deletions(-)
 delete mode 100644 HACKING.rst

diff --git a/CODING_STYLE.rst b/CODING_STYLE.rst
index 713357cb80..4501d87352 100644
--- a/CODING_STYLE.rst
+++ b/CODING_STYLE.rst
@@ -205,6 +205,302 @@ comment anyway.)
 Rationale: Consistency, and ease of visually picking out a multiline
 comment from the surrounding code.
 
+Preprocessor
+
+
+Variadic macros
+---
+
+For variadic macros, stick with this C99-like syntax:
+
+.. code-block:: c
+
+#define DPRINTF(fmt, ...)   \
+do { printf("IRQ: " fmt, ## __VA_ARGS__); } while (0)
+
+Include directives
+--
+
+Order include directives as follows:
+
+.. code-block:: c
+
+#include "qemu/osdep.h"  /* Always first... */
+#include <...>   /* then system headers... */
+#include "..."   /* and finally QEMU headers. */
+
+The "qemu/osdep.h" header contains preprocessor macros that affect the behavior
+of core system headers like .  It must be the first include so that
+core system headers included by external libraries get the preprocessor macros
+that QEMU depends on.
+
+Do not include "qemu/osdep.h" from header files since the .c file will have
+already included it.
+
+C types
+===
+
+It should be common sense to use the right type, but we have collected
+a few useful guidelines here.
+
+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.
+
+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 hwaddr for guest physical addresses except pcibus_t
+for PCI addresses.  In addition, 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.
+
+For CPU virtual addresses there are several possible types.
+vaddr is the best type to use to hold a CPU virtual address in
+target-independent code. It is guaranteed to be large enough to hold a
+virtual address for any target, and it does not change size from target
+to target. It is always unsigned.
+target_ulong is a type the size of a virtual address on the CPU; this means
+it may be 32 or 64 bits depending on which target is being built. It should
+therefore be used only in target-specific code, and in some
+performance-critical built-per-target core code such as the TLB code.
+There is also a signed version, target_long.
+abi_ulong is for the ``*``-user targets, and represents a type the size of
+'void ``*``' in that target's ABI. (This may not be the same as the size of a
+full CPU virtual address in the case of target ABIs which use 32 bit pointers
+on 64 bit CPUs, like sparc32plus.) Definitions of structures that must match
+the target's ABI must use this type for anything that on the target is defined
+to be an 'unsigned long' or a pointer type.
+There is also a signed version, abi_long.
+
+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.
+
+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" attribut