Re: [Qemu-devel] [PATCH 2/4] docs: merge HACKING.md contents into CODING_STYLE.md
On Wed, Aug 28, 2019 at 04:06:20PM +0100, Alex Bennée wrote: > > Eric Blake writes: > > > On 8/23/19 11:39 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.md | 262 +++ > >> HACKING.md | 263 > >> README | 2 +- > >> 3 files changed, 263 insertions(+), 264 deletions(-) > >> delete mode 100644 HACKING.md > > > > Is it worth trying to group related sections as part of the combination? > > (Your solution of just concatenating at the end is obviously the > > fastest, but may result in odd ordering where similar things are > > mentioned twice but in different parts of the file). > > It is a bit all over the place, but just moving trace-events and > automatic memory de-allocation we could group it like this: > > Formatting and style: > > * [Whitespace](#whitespace) > + [Multiline Indent](#multiline-indent) > * [Line width](#line-width) > * [Naming](#naming) > * [Block structure](#block-structure) > * [Declarations](#declarations) > * [Conditional statements](#conditional-statements) > * [Comment style](#comment-style) > > Language usage: > > * [Preprocessor](#preprocessor) > + [Variadic macros](#variadic-macros) > + [Include directives](#include-directives) > * [C types](#c-types) > + [Scalars](#scalars) > + [Pointers](#pointers) > + [Typedefs](#typedefs) > + [Reserved namespaces in C and > POSIX](#reserved-namespaces-in-c-and-posix) > * [Low level memory management](#low-level-memory-management) > * [String manipulation](#string-manipulation) > * [Printf-style functions](#printf-style-functions) > * [C standard, implementation defined and undefined > behaviors](#c-standard--implementation-defined-and-undefined-behaviors) > * [Automatic memory deallocation](#automatic-memory-deallocation) > > QEMU Specific Idioms > > * [trace-events style](#trace-events-style) > + [0x prefix](#0x-prefix) > + ['#' printf flag](#printf-flag) > * [Error handling and reporting](#error-handling-and-reporting) > + [Reporting errors to the human > user](#reporting-errors-to-the-human-user) > + [Propagating errors](#propagating-errors) > + [Handling errors](#handling-errors) Sure, I'm fine doing this. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 2/4] docs: merge HACKING.md contents into CODING_STYLE.md
Eric Blake writes: > On 8/23/19 11:39 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.md | 262 +++ >> HACKING.md | 263 >> README | 2 +- >> 3 files changed, 263 insertions(+), 264 deletions(-) >> delete mode 100644 HACKING.md > > Is it worth trying to group related sections as part of the combination? > (Your solution of just concatenating at the end is obviously the > fastest, but may result in odd ordering where similar things are > mentioned twice but in different parts of the file). It is a bit all over the place, but just moving trace-events and automatic memory de-allocation we could group it like this: Formatting and style: * [Whitespace](#whitespace) + [Multiline Indent](#multiline-indent) * [Line width](#line-width) * [Naming](#naming) * [Block structure](#block-structure) * [Declarations](#declarations) * [Conditional statements](#conditional-statements) * [Comment style](#comment-style) Language usage: * [Preprocessor](#preprocessor) + [Variadic macros](#variadic-macros) + [Include directives](#include-directives) * [C types](#c-types) + [Scalars](#scalars) + [Pointers](#pointers) + [Typedefs](#typedefs) + [Reserved namespaces in C and POSIX](#reserved-namespaces-in-c-and-posix) * [Low level memory management](#low-level-memory-management) * [String manipulation](#string-manipulation) * [Printf-style functions](#printf-style-functions) * [C standard, implementation defined and undefined behaviors](#c-standard--implementation-defined-and-undefined-behaviors) * [Automatic memory deallocation](#automatic-memory-deallocation) QEMU Specific Idioms * [trace-events style](#trace-events-style) + [0x prefix](#0x-prefix) + ['#' printf flag](#printf-flag) * [Error handling and reporting](#error-handling-and-reporting) + [Reporting errors to the human user](#reporting-errors-to-the-human-user) + [Propagating errors](#propagating-errors) + [Handling errors](#handling-errors) -- Alex Bennée
Re: [Qemu-devel] [PATCH 2/4] docs: merge HACKING.md contents into CODING_STYLE.md
On 8/23/19 11:39 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.md | 262 +++ > HACKING.md | 263 > README | 2 +- > 3 files changed, 263 insertions(+), 264 deletions(-) > delete mode 100644 HACKING.md Is it worth trying to group related sections as part of the combination? (Your solution of just concatenating at the end is obviously the fastest, but may result in odd ordering where similar things are mentioned twice but in different parts of the file). -- 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 2/4] docs: merge HACKING.md contents into CODING_STYLE.md
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.md | 262 +++
HACKING.md | 263
README | 2 +-
3 files changed, 263 insertions(+), 264 deletions(-)
delete mode 100644 HACKING.md
diff --git a/CODING_STYLE.md b/CODING_STYLE.md
index 056eda7739..9f4fc9dc77 100644
--- a/CODING_STYLE.md
+++ b/CODING_STYLE.md
@@ -217,3 +217,265 @@ and '%#...'. For consistency the only one way should be
used. Arguments for
- it is more popular
- '%#' omits the 0x for the value 0 which makes output inconsistent
+
+
+## Preprocessor
+
+### Variadic macros
+
+For variadic macros, stick with this C99-like syntax:
+
+#define DPRINTF(fmt, ...) \
+do { printf("IRQ: " fmt, ## __VA_ARGS__); } while (0)
+
+### Include directives
+
+Order include directives as follows:
+
+#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" attribute. That way, the reader knows
+up-front that this is a
