Although we have nothing in make syntax-check to enforce this, and
apparently there are places where it isn't the case (according to
Dan), we should discourage the practice of defining new variables in
the middle of a block of code.

https://www.redhat.com/archives/libvir-list/2020-July/msg00433.html
Signed-off-by: Laine Stump <laine@redhat.com>
---
 docs/coding-style.rst | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/docs/coding-style.rst b/docs/coding-style.rst
index 03b89c86e5..b9b4a16987 100644
--- a/docs/coding-style.rst
+++ b/docs/coding-style.rst
@@ -541,6 +541,44 @@ diligent about this, when you see a non-const pointer, you're
 guaranteed that it is used to modify the storage it points to, or
 it is aliased to another pointer that is.
 
+Defining Local Variables
+------------------------
+
+Always define local variables at the top of the block in which they
+are used (before any pure code). Although modern C compilers allow
+defining a local variable in the middle of a block of code, this
+practice can lead to bugs, and must be avoided in all libvirt
+code. (As indicated in these examples, it is okay to initialize
+variables where they are defined, even if the initialization involves
+calling another function.)
+
+::
+
+  GOOD:
+    int
+    Bob(char *loblaw)
+    {
+        int x;
+        int y = lawBlog(loblaw);
+        char *z = NULL;
+
+        x = y + 20;
+        ...
+    }
+
+  BAD:
+    int
+    Bob(char *loblaw)
+    {
+        int x;
+        int y = lawBlog(loblaw);
+
+        x = y + 20;
+
+        char *z = NULL; <===
+        ...
+    }
+
 Attribute annotations
 ---------------------
 
-- 
2.25.4

On Thu, Jul 09, 2020 at 06:41:21PM -0400, Laine Stump wrote:
> Although we have nothing in make syntax-check to enforce this, and
> apparently there are places where it isn't the case (according to
> Dan), we should discourage the practice of defining new variables in
> the middle of a block of code.
> 
> https://www.redhat.com/archives/libvir-list/2020-July/msg00433.html
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>  docs/coding-style.rst | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|

On Thu, 2020-07-09 at 18:41 -0400, Laine Stump wrote:
> +Defining Local Variables
> +------------------------
> +
> +Always define local variables at the top of the block in which they
> +are used (before any pure code). Although modern C compilers allow
> +defining a local variable in the middle of a block of code, this
> +practice can lead to bugs, and must be avoided in all libvirt
> +code. (As indicated in these examples, it is okay to initialize
> +variables where they are defined, even if the initialization involves
> +calling another function.)

The parentheses around the last sentence are unnecessary, please
drop them.

> +  GOOD:
> +    int
> +    Bob(char *loblaw)
> +    {
> +        int x;
> +        int y = lawBlog(loblaw);

I believe this should be

  int y = lawBlog();

but note that I haven't compile-tested this alternative version.

> +  BAD:
> +    int
> +    Bob(char *loblaw)
> +    {
> +        int x;
> +        int y = lawBlog(loblaw);
> +
> +        x = y + 20;
> +
> +        char *z = NULL; <===

Please add // in front of the ASCII arrow.

It's pretty weird how we use C++-style comments throughout our style
guide, at the same time as *the style guide itself* instructs
developers to use C-style comments instead, but addressing that is a
job for another patch :)


With the nits fixed,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

On Thu, Jul 09, 2020 at 06:41:21PM -0400, Laine Stump wrote:
> Although we have nothing in make syntax-check to enforce this, and
> apparently there are places where it isn't the case (according to
> Dan), we should discourage the practice of defining new variables in
> the middle of a block of code.
> 
> https://www.redhat.com/archives/libvir-list/2020-July/msg00433.html
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>  docs/coding-style.rst | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/docs/coding-style.rst b/docs/coding-style.rst
> index 03b89c86e5..b9b4a16987 100644
> --- a/docs/coding-style.rst
> +++ b/docs/coding-style.rst
> @@ -541,6 +541,44 @@ diligent about this, when you see a non-const pointer, you're
>  guaranteed that it is used to modify the storage it points to, or
>  it is aliased to another pointer that is.
>  
> +Defining Local Variables
> +------------------------
> +
> +Always define local variables at the top of the block in which they
> +are used (before any pure code). Although modern C compilers allow
> +defining a local variable in the middle of a block of code, this
> +practice can lead to bugs, and must be avoided in all libvirt
> +code. (As indicated in these examples, it is okay to initialize
> +variables where they are defined, even if the initialization involves
> +calling another function.)
> +
> +::
> +
> +  GOOD:
> +    int
> +    Bob(char *loblaw)

Since we are nitpicking I don't think we allow the first letter of the
function name to be uppercase. :)

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>