Re: [PATCH 2/3] docs: hacking: Add good practices for shortening conditional expressions

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

 



On Thu, May 09, 2019 at 12:43:33PM +0200, Peter Krempa wrote:
Document that checking if a integer is (non-)zero should (not must)
avoid the shortened form that C allows as it may confuse readers into
overlooking the other possible values which might be interresting to
handle.

While pointers have distinct values from the point of view of the code
we only care whether it's non-NULL and thus it's documented it's okay
to shorten those.

Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
---
docs/hacking.html.in | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 081d793360..a2800853ef 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -826,6 +826,28 @@
  }
</pre>

+    <h2><a id="conditions">Conditional expressions</a></h2>
+      <p>For readability reasons new code should avoid shortening comparisons
+        to 0 for numeric types. Boolean and pointer comparisions may be
+        shortened. All long forms are okay:
+      </p>
+<pre>
+   virFooPtr foos = NULL;
+   size nfoos = 0;
+   bool hasFoos = false;
+
+GOOD:
+    if (!foos)
+    if (!hasFoos)
+    if (nfoos == 0)
+    if (foos == NULL)
+    if (hasFoos == true)
+
+BAD:
+    if (!nfoos)
+    if (foos)

why is this bad when it is a pointer?  Typo?

+</pre>
+
    <h2><a id="preprocessor">Preprocessor</a></h2>

    <p>Macros defined with an ALL_CAPS name should generally be
--
2.21.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux