On a Friday in 2022, Tim Wiederhake wrote: This weakly suggests that they should be introduced for new public types. I have no preference, but I think we should mention it here if we're mentioning internal types. Jano

The code style showed `bool hasFoos; if (hasFoos == true)` as a good example in one place, only to warn against comparisons with `true` a couple of paragraphs further down. Merge this advice on comparing with `true` into the "Conditional expressions" section and split the example up for readability. Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; --- docs/coding-style.rst | 60 +++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/docs/coding-style.rst b/docs/coding-style.rst index dca9de1915..3dedb032f4 100644 --- a/docs/coding-style.rst +++ b/docs/coding-style.rst @@ -427,25 +427,47 @@ Conditional expressions ----------------------- For readability reasons new code should avoid shortening -comparisons to 0 for numeric types. Boolean and pointer -comparisons may be shortened. All long forms are okay: +comparisons to 0 for numeric types: :: - virFoo *foos = NULL; size nfoos = 0; - bool hasFoos = false; GOOD: - if (!foos) - if (!hasFoos) + if (nfoos != 0) if (nfoos == 0) - if (foos == NULL) - if (hasFoos == true) BAD: - if (!nfoos) if (nfoos) + if (!nfoos) + +Prefer the shortened version for boolean values. Boolean values +should never be compared against the literal ``true``, as a +logical non-false value need not be ``1``. + +:: + + bool hasFoos = false; + + GOOD: + if (hasFoos) + if (!hasFoos) + + BAD: + if (hasFoos == true) + if (hasFoos != false) + if (hasFoos == false) + if (hasFoos != true) + +Pointer comparisons may be shortened. All long forms are okay. + +:: + + virFoo *foo = NULL; + + GOOD: + if (foo) # or: if (foo != NULL) + if (!foo) # or: if (foo == NULL) New code should avoid the ternary operator as much as possible. Specifically it must never span more than one line or nest: @@ -507,19 +529,13 @@ Scalars - In the unusual event that you require a specific width, use a standard type like ``int32_t``, ``uint32_t``, ``uint64_t``, etc. -- 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. Note that it would be possible - (albeit wasteful) to use ``bool`` in libvirt's logical wire - protocol, since XDR maps that to its lower-level ``bool_t`` - type, which **is** fixed-size. - - 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)...``. +- While using ``bool`` is good for readability, it comes with a + minor caveat: Don't use ``bool`` in places where the type size + must be constant across all systems, like public interfaces and + on-the-wire protocols. Note that it would be possible (albeit + wasteful) to use ``bool`` in libvirt's logical wire protocol, + since XDR maps that to its lower-level ``bool_t`` type, which + **is** fixed-size. 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 -- 2.31.1

There are no instances of that label left. Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; --- docs/coding-style.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/coding-style.rst b/docs/coding-style.rst index 3dedb032f4..14c5136398 100644 --- a/docs/coding-style.rst +++ b/docs/coding-style.rst @@ -953,7 +953,6 @@ makes sense: error: A path only taken upon return with an error code cleanup: A path taken upon return with success code + optional error - no_memory: A path only taken upon return with an OOM error code retry: If needing to jump upwards (e.g., retry on EINTR) Top-level labels should be indented by one space (putting them on -- 2.31.1

This was not mentioned before. Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com&gt; --- docs/coding-style.rst | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/coding-style.rst b/docs/coding-style.rst index 14c5136398..e1ed34f764 100644 --- a/docs/coding-style.rst +++ b/docs/coding-style.rst @@ -600,6 +600,19 @@ calling another function. ... } +Define variables on separate lines. This allows for smaller, easier to +understand diffs when changing them. Define variables in the smallest +possible scope. + +:: + + GOOD: + int x; + int y; + + BAD: + int x, y; + Attribute annotations --------------------- -- 2.31.1

On 1/14/22 10:56 AM, Ján Tomko wrote: I also don't mind combining simple things like that, but am willing to go full-isolated just for consistency's sake. Since it's Friday and we're talking about personal preferences - I personally dislike the use of i and j (and anything else with a single letter) as variable names, because it makes using a text search for occurences pointless. Sure, longer variable names could also be a substring of something else, and any variable could be re-used elsewhere, but even then a search is mildly usable. (On the other hand, sometimes a loop is just a loop and it takes too much brain capacity to think of a meaningful name for the index. I used to work with someone who always used "ii" and "jj" for generic loop indexes because they were then easy to search for with few false positives (well - "ascii", "skiing", and a surprisingly high number of other more obscure words, but still...) , and I internalized that practice myself. After having libvirt patches with that rejected a couple times, I unlearned and conformed to the hive :-))