5:43 a.m.
Peter Krempa (3):
docs: hacking: Document few practices for creating error messages
docs: hacking: Add good practices for shortening conditional
expressions
docs: hacking: Discourage use of the ternary operator and ban it's
abuse
docs/hacking.html.in | 61 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
--
2.21.0
5:43 a.m.
New subject: [libvirt] [PATCH 1/3] docs: hacking: Document few practices for creating error messages
State that error messages should not be broken into multiple lines for
programmer friendliness and should not be concatenated on the fly for
translator friendliness and few other details.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/hacking.html.in | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 902d05430f..081d793360 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -1284,6 +1284,34 @@
does for snprintf.
</p>
+ <h2><a id="errors">Error message format</a></h2>
+
+ <p>
+ Error messages visible to the user should be short and descriptive. All
+ error messages are translated using gettext and thus must be wrapped in
+ <code>_()</code> macro. To simplify the translation work, the error
message
+ must not be concatenated from various parts. To simplify searching for
+ the error message in the code the strings should not be broken even
+ if they result into a line longer than 80 columns and any formatting
+ modifier should be enclosed by quotes or other obvious separator.
+ If a string used with <code>%s</code> can be NULL the NULLSTR macro
must
+ be used.
+ </p>
+
+<pre>
+ GOOD: virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to connect to remote host '%s'"),
hostname)
+
+ BAD: virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to %s to remote host '%s'"),
+ "connect", hostname);
+
+ BAD: virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to connect "
+ "to remote host '%s'),
+ hostname);
+</pre>
+
<h2><a id="goto">Use of goto</a></h2>
<p>
--
2.21.0
5:43 a.m.
New subject: [libvirt] [PATCH 2/3] docs: hacking: Add good practices for shortening conditional expressions
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(a)redhat.com>
---
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)
+</pre>
+
<h2><a id="preprocessor">Preprocessor</a></h2>
<p>Macros defined with an ALL_CAPS name should generally be
--
2.21.0
5:53 a.m.
New subject: [libvirt] [PATCH 2/3] docs: hacking: Add good practices for shortening conditional expressions
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(a)redhat.com>
---
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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
6:07 a.m.
New subject: [libvirt] [PATCH 2/3] docs: hacking: Add good practices for shortening conditional expressions
On Thu, May 09, 2019 at 12:53:07 +0200, Martin Kletzander wrote:
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(a)redhat.com>
> ---
> 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?
Oops, yes. This was supposed to be "if (nfoos)"
5:43 a.m.
New subject: [libvirt] [PATCH 3/3] docs: hacking: Discourage use of the ternary operator and ban it's abuse
Forbid breaking lines inside the two branches of the ternary operator
and nesting them. Using it in these instances does not help readability.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
docs/hacking.html.in | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index a2800853ef..fc4adae354 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -847,6 +847,17 @@ BAD:
if (!nfoos)
if (foos)
</pre>
+ <p>New code should avoid the ternary operator as much as possible.
+ Specifically it must never span more than one line or nest:
+ </p>
+<pre>
+BAD:
+ char *foo = baz ?
+ virDoSomethingReallyComplex(driver, vm, something, baz->foo) :
+ NULL;
+
+ char *foo = bar ? bar->baz ? bar->baz->foo : "nobaz" :
"nobar";
+</pre>
<h2><a id="preprocessor">Preprocessor</a></h2>
--
2.21.0
7:52 a.m.
On Thu, May 09, 2019 at 12:43:31 +0200, Peter Krempa wrote:
Peter Krempa (3):
docs: hacking: Document few practices for creating error messages
docs: hacking: Add good practices for shortening conditional
expressions
docs: hacking: Discourage use of the ternary operator and ban it's
abuse
docs/hacking.html.in | 61 ++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 61 insertions(+)
Ping?
8:26 a.m.
On 5/21/19 7:52 AM, Peter Krempa wrote:
On Thu, May 09, 2019 at 12:43:31 +0200, Peter Krempa wrote:
> Peter Krempa (3):
> docs: hacking: Document few practices for creating error messages
> docs: hacking: Add good practices for shortening conditional
> expressions
> docs: hacking: Discourage use of the ternary operator and ban it's
> abuse
>
> docs/hacking.html.in | 61 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 61 insertions(+)
Ping?
More work for me on my incremental backup series, but I can agree to all
three points.
Once you fix the typo,
ACK series
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
2002
days inactive
2014
days old
7 comments
3 participants
participants (3)
-
Eric Blake -
Martin Kletzander -
Peter Krempa