[SCM] Samba Shared Repository - branch master updated

Michael Adam obnox at samba.org
Wed May 30 05:26:10 MDT 2012


Volker Lendecke wrote:
> On Wed, May 30, 2012 at 11:25:59AM +0200, David Disseldorp wrote:
> > On Wed, 30 May 2012 07:59:22 +0200
> > Volker Lendecke <Volker.Lendecke at SerNet.DE> wrote:
> > 
> > > Doing a return or
> > > goto from something that looks like a function call is just
> > > wrong to me. To save precious screen space, I would rather
> > > go and introduce a special rule to say
> > > 
> > > if (ptr == NULL) { return WERR_NOMEM };
> > > 
> > > in one line. I know we have tons of uses of those, but I
> > > would like to start a discussion about banning them.
> > 
> > +1 from me, macros that effect control flow are evil IMO.
> > 
> > My preference would be to use a two line if statement:
> > if (ptr == NULL)
> > 	 return WERR_NOMEM;

Uh, for me this is even far worse than the one-line statment
above. IMHO We should not do this kind of indentation without
braces. It is too error-prone. Too easy to mis-read.

> If nobody objects, I will push the attached patch by the end
> of this week.

OK for me.

Cheers - Michael


> >From 3e53b8da6f1a0b56b1d32afdb7e77b173ce8ef14 Mon Sep 17 00:00:00 2001
> From: Volker Lendecke <vl at samba.org>
> Date: Wed, 30 May 2012 10:14:51 +0200
> Subject: [PATCH] Coding: Add comment disproving control-flow changing macros
> 
> ---
>  README.Coding |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/README.Coding b/README.Coding
> index 8416290..956a733 100644
> --- a/README.Coding
> +++ b/README.Coding
> @@ -367,3 +367,13 @@ Bad Example:
>  	ret = some_function_my_name(get_some_name());
>  	...
>  
> +Control-Flow changing macros
> +----------------------------
> +
> +Macros like NT_STATUS_NOT_OK_RETURN that change control flow
> +(return/goto/etc) from within the macro are considered bad, because
> +they look like function calls that never change control flow. Please
> +do not use them in new code.
> +
> +The only exception is the test code that depends repeated use of calls
> +like CHECK_STATUS, CHECK_VAL and others.
> -- 
> 1.7.8
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120530/52ce2c76/attachment.pgp>


More information about the samba-technical mailing list