Re: [PATCH] docs: point out that locals should be defined at the top of a block of code

2020-07-10 Thread Pavel Hrdina 
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 
> ---
>  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 


signature.asc
Description: PGP signature

Re: [PATCH] docs: point out that locals should be defined at the top of a block of code

2020-07-10 Thread Daniel P . Berrangé 
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 
> ---
>  docs/coding-style.rst | 38 ++
>  1 file changed, 38 insertions(+)

Reviewed-by: Daniel P. Berrangé 


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: [PATCH] docs: point out that locals should be defined at the top of a block of code

2020-07-10 Thread Andrea Bolognani 
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 

-- 
Andrea Bolognani / Red Hat / Virtualization

[PATCH] docs: point out that locals should be defined at the top of a block of code

2020-07-09 Thread Laine Stump 
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 
---
 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