Mailing List Archive

[PATCH RFC v2 06/13] libxl: permit declaration after statement
GCC and C99 allow declarations to be mixed with code. This is a good
idea because:

* It allows variables to be more often initialised as they are
declared, thus reducing the occurrence of uninitialised variable
errors.

* Certain alloca-like constructs (arrays allocated at runtime on the
stack) can more often be written without a spurious { } block.
Such blocks are confusing to read.

* It makes it easier to write and use macros which declare and
initialise formulaic variables and do other function setup code,
because there is no need to worry that such macros might be
incompatible with each other or have strict ordering constraints.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
tools/libxl/Makefile | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index f23661a..a3727ab 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -11,7 +11,8 @@ MINOR = 0
XLUMAJOR = 1.0
XLUMINOR = 0

-CFLAGS += -Werror -Wno-format-zero-length -Wmissing-declarations
+CFLAGS += -Werror -Wno-format-zero-length -Wmissing-declarations \
+ -Wno-declaration-after-statement
CFLAGS += -I. -fPIC

ifeq ($(CONFIG_Linux),y)
--
1.7.2.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH RFC v2 06/13] libxl: permit declaration after statement [ In reply to ]
On Fri, 2011-10-28 at 19:37 +0100, Ian Jackson wrote:
> GCC and C99 allow declarations to be mixed with code. This is a good
> idea because:
>
> * It allows variables to be more often initialised as they are
> declared, thus reducing the occurrence of uninitialised variable
> errors.
>
> * Certain alloca-like constructs (arrays allocated at runtime on the
> stack) can more often be written without a spurious { } block.
> Such blocks are confusing to read.
>
> * It makes it easier to write and use macros which declare and
> initialise formulaic variables and do other function setup code,
> because there is no need to worry that such macros might be
> incompatible with each other or have strict ordering constraints.
>
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>

I suspect there isn't much scope for abuse of this capability but would
a few words of guidance in CODING_STYLE make sense?

> ---
> tools/libxl/Makefile | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
> index f23661a..a3727ab 100644
> --- a/tools/libxl/Makefile
> +++ b/tools/libxl/Makefile
> @@ -11,7 +11,8 @@ MINOR = 0
> XLUMAJOR = 1.0
> XLUMINOR = 0
>
> -CFLAGS += -Werror -Wno-format-zero-length -Wmissing-declarations
> +CFLAGS += -Werror -Wno-format-zero-length -Wmissing-declarations \
> + -Wno-declaration-after-statement
> CFLAGS += -I. -fPIC
>
> ifeq ($(CONFIG_Linux),y)



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH RFC v2 06/13] libxl: permit declaration after statement [ In reply to ]
Ian Campbell writes ("Re: [Xen-devel] [PATCH RFC v2 06/13] libxl: permit declaration after statement"):
> I suspect there isn't much scope for abuse of this capability but would
> a few words of guidance in CODING_STYLE make sense?

I don't think there's anything I feel needs saying. If you think
people are likely to abuse it in a particular way then do say :-).

Otherwise we can wait and see what results and add stuff to
CODING_STYLE as necessary ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH RFC v2 06/13] libxl: permit declaration after statement [ In reply to ]
On Wed, 2011-11-02 at 12:36 -0400, Ian Jackson wrote:
> Ian Campbell writes ("Re: [Xen-devel] [PATCH RFC v2 06/13] libxl: permit declaration after statement"):
> > I suspect there isn't much scope for abuse of this capability but would
> > a few words of guidance in CODING_STYLE make sense?
>
> I don't think there's anything I feel needs saying. If you think
> people are likely to abuse it in a particular way then do say :-).
>
> Otherwise we can wait and see what results and add stuff to
> CODING_STYLE as necessary ?

Yes, lets do that.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH RFC v2 06/13] libxl: permit declaration after statement [ In reply to ]
On Wed, 2 Nov 2011, Ian Campbell wrote:
> On Wed, 2011-11-02 at 12:36 -0400, Ian Jackson wrote:
> > Ian Campbell writes ("Re: [Xen-devel] [PATCH RFC v2 06/13] libxl: permit declaration after statement"):
> > > I suspect there isn't much scope for abuse of this capability but would
> > > a few words of guidance in CODING_STYLE make sense?
> >
> > I don't think there's anything I feel needs saying. If you think
> > people are likely to abuse it in a particular way then do say :-).
> >
> > Otherwise we can wait and see what results and add stuff to
> > CODING_STYLE as necessary ?
>
> Yes, lets do that.
>

I think it would make sense to add to the CODING_STYLE that variable
declarations shouldn't be mixed with code, unless part of a macro or an
alloca-like construct.
Basically the coding style should stay the same that is now.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH RFC v2 06/13] libxl: permit declaration after statement [ In reply to ]
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH RFC v2 06/13] libxl: permit declaration after statement"):
> I think it would make sense to add to the CODING_STYLE that variable
> declarations shouldn't be mixed with code, unless part of a macro or an
> alloca-like construct.

I think this is a silly rule. In my proposed commit message I
advanced three reasons for allowing declaration after statement:

> > * It allows variables to be more often initialised as they are
> > declared, thus reducing the occurrence of uninitialised variable
> > errors.
> >
> > * Certain alloca-like constructs (arrays allocated at runtime on the
> > stack) can more often be written without a spurious { } block.
> > Such blocks are confusing to read.
> >
> > * It makes it easier to write and use macros which declare and
> > initialise formulaic variables and do other function setup code,
> > because there is no need to worry that such macros might be
> > incompatible with each other or have strict ordering constraints.

Of these the first two improvements would be banned by your proposed
coding style rule.

I don't understand what the harm is in allowing declarations, with
initialisation, freely mixed with code.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH RFC v2 06/13] libxl: permit declaration after statement [ In reply to ]
On Mon, 2011-11-28 at 16:40 +0000, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH RFC v2 06/13] libxl: permit declaration after statement"):
> > I think it would make sense to add to the CODING_STYLE that variable
> > declarations shouldn't be mixed with code, unless part of a macro or an
> > alloca-like construct.
>
> I think this is a silly rule. In my proposed commit message I
> advanced three reasons for allowing declaration after statement:
>
> > > * It allows variables to be more often initialised as they are
> > > declared, thus reducing the occurrence of uninitialised variable
> > > errors.
> > >
> > > * Certain alloca-like constructs (arrays allocated at runtime on the
> > > stack) can more often be written without a spurious { } block.
> > > Such blocks are confusing to read.
> > >
> > > * It makes it easier to write and use macros which declare and
> > > initialise formulaic variables and do other function setup code,
> > > because there is no need to worry that such macros might be
> > > incompatible with each other or have strict ordering constraints.
>
> Of these the first two improvements would be banned by your proposed
> coding style rule.
>
> I don't understand what the harm is in allowing declarations, with
> initialisation, freely mixed with code.

Variable declarations should either be at the top of the function or
immediately before / as part of the block which uses them. e.g. this is
unhelpful:

void a_func(void)
{
/* a bunch of code; */

int bar;

/* a bunch more code not using bar */

bar = get_me_a_bar();

/* use bar lots */
}

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH RFC v2 06/13] libxl: permit declaration after statement [ In reply to ]
On Tue, 2011-11-29 at 10:45 +0000, Ian Campbell wrote:

> Variable declarations should either be at the top of the function or
> immediately before / as part of the block which uses them. e.g. this is
> unhelpful:

This isn't an objection to the patch BTW, I'm happy with your
reasoning...

>
> void a_func(void)
> {
> /* a bunch of code; */
>
> int bar;
>
> /* a bunch more code not using bar */
>
> bar = get_me_a_bar();
>
> /* use bar lots */
> }
>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH RFC v2 06/13] libxl: permit declaration after statement [ In reply to ]
Ian Campbell writes ("Re: [Xen-devel] [PATCH RFC v2 06/13] libxl: permit declaration after statement"):
> Variable declarations should either be at the top of the function or
> immediately before / as part of the block which uses them. e.g. this is
> unhelpful:

I agree..

> void a_func(void)
> {
> /* a bunch of code; */
>
> int bar;
>
> /* a bunch more code not using bar */
>
> bar = get_me_a_bar();
>
> /* use bar lots */
> }

Yes, but would anyone really write that ? I think the argument is
about this:

/* a bunch of code; */

int bar = get_me_a_bar();

/* use bar lots */

I think this is fine and I think Stefano objects.

How about:

Declarations of automatic variables should either be at the start of
the relevant function (or block), or as late as reasonable; in the
latter case they should be initialised in the declaration if
possible.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH RFC v2 06/13] libxl: permit declaration after statement [ In reply to ]
On Mon, 28 Nov 2011, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH RFC v2 06/13] libxl: permit declaration after statement"):
> > I think it would make sense to add to the CODING_STYLE that variable
> > declarations shouldn't be mixed with code, unless part of a macro or an
> > alloca-like construct.
>
> I think this is a silly rule. In my proposed commit message I
> advanced three reasons for allowing declaration after statement:
>
> > > * It allows variables to be more often initialised as they are
> > > declared, thus reducing the occurrence of uninitialised variable
> > > errors.
> > >
> > > * Certain alloca-like constructs (arrays allocated at runtime on the
> > > stack) can more often be written without a spurious { } block.
> > > Such blocks are confusing to read.
> > >
> > > * It makes it easier to write and use macros which declare and
> > > initialise formulaic variables and do other function setup code,
> > > because there is no need to worry that such macros might be
> > > incompatible with each other or have strict ordering constraints.
>
> Of these the first two improvements would be banned by your proposed
> coding style rule.

Only the first would be banned, I am OK with making an exception for
alloca constructs and macros.


> I don't understand what the harm is in allowing declarations, with
> initialisation, freely mixed with code.


It makes the code harder to read;
it makes it more difficult to rearrange local variables in the future;
it makes it more difficult to see how much stack your function is using;
it makes it more difficult to realize if you can reduce the amount of
local variables you are using.

And it violates the current coding style.

I think that declaring variables at the beginning of the function is a
good programming practice in any language.

The three most important C codebases in the Xen project are: Linux,
Qemu and Xen. None of these allow mixing declarations and code, for a
good reason. I don't think libxl should have a different code style in
this regard, it would just be confusing.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH RFC v2 06/13] libxl: permit declaration after statement [ In reply to ]
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH RFC v2 06/13] libxl: permit declaration after statement"):
> On Mon, 28 Nov 2011, Ian Jackson wrote:
> > I don't understand what the harm is in allowing declarations, with
> > initialisation, freely mixed with code.
>
>
> It makes the code harder to read;

Naturally I disagree, but this is a matter of subjective taste as far
as I can tell, unless you have something specific to point to.

> it makes it more difficult to rearrange local variables in the future;

I'm not sure what you mean by "rearrange local variables". The style
where variables are declared only at the top of the file tends to
result in long lists of local variables in declaration statements at
the top of functions. Those long lists make editing the function
somewhat more complex.

Declaring variables in the same statement as they are initialised
naturally makes "rearranging" them trivial.

> it makes it more difficult to see how much stack your function is using;

This is not relevant in libxl unless the objects are truly huge (in
which case they shouldn't be on the stack at all).

> it makes it more difficult to realize if you can reduce the amount of
> local variables you are using.

There is no benefit in trying to "reduce the amount of local
variables" in userland C code compiled with a reasonable optimising
compiler. The compiler will be able to do the same liveness analysis
either way.

> And it violates the current coding style.

This is not an argument against changing the coding style.

> I think that declaring variables at the beginning of the function is a
> good programming practice in any language.

I think that initialising variables at the time they are declared,
where reasonable, is good programming practice in any language.

> The three most important C codebases in the Xen project are: Linux,
> Qemu and Xen. None of these allow mixing declarations and code, for a
> good reason. I don't think libxl should have a different code style in
> this regard, it would just be confusing.

I don't see this as a particularly relevant consideration. There are
lots of other ways our coding style differs from (say) Linux. Any
competent C programmer will be familiar with both styles.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel