Coding guidelines change (proposed)

I'd like to propose a change to the coding guidelines, and
I'm airing it here
for specific feedback before we make this change. 
Currently, our standard
is that if you have a single statement following an
if/for/while construct,
then we do not require parentheses around the single
statement.  While this
is legal C, it has caused a number of issues in the past,
where another
statement was added at that indentation level, but without
adding the
necessary braces.

I'd like to change the coding guidelines that the braces are
no longer
optional; that they are mandatory, and if this change is
approved, a janitor
project will be posted shortly to convert legacy code to the
new standard.

Are there any comments or objections before we implement
this change?

-- 
Tilghman

_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

I am not an active asterisk developer but I think requiring
that is a  
great idea - I have used that style in c and other languages
for over  
20yrs for exactly the same reasons.



Quoting Tilghman Lesher <tilghmanmail.jeffandtilghman.com>:

> I'd like to propose a change to the coding guidelines,
and I'm airing it here
> for specific feedback before we make this change. 
Currently, our standard
> is that if you have a single statement following an
if/for/while construct,
> then we do not require parentheses around the single
statement.  While this
> is legal C, it has caused a number of issues in the
past, where another
> statement was added at that indentation level, but
without adding the
> necessary braces.
>
> I'd like to change the coding guidelines that the
braces are no longer
> optional; that they are mandatory, and if this change
is approved, a janitor
> project will be posted shortly to convert legacy code
to the new standard.
>
> Are there any comments or objections before we
implement this change?
>
> --
> Tilghman
>
> _______________________________________________
> --Bandwidth and Colocation Provided by http://www.api-digital.c
om--
>
> asterisk-dev mailing list
> To UNSUBSCRIBE or update options visit:
>    http://lists.digium.com/mailman/listinfo/asterisk-dev
>
>



Jon Pounder

    _/_/_/  _/    _/  _/       _/_/_/  _/    _/  _/_/_/_/
     _/    _/_/  _/  _/         _/    _/_/  _/  _/_/
    _/    _/  _/_/  _/         _/    _/  _/_/  _/
_/_/_/  _/    _/  _/_/_/_/ _/_/_/  _/    _/  _/_/_/_/


Inline Internet Systems Inc.
Thorold, Ontario, Canada

Tools to Power Your e-Business Solutions
www.inline.net
www.ihtml.com
www.ihtmlmerchant.com
www.opayc.com



_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

On Tue, 2008-02-26 at 11:46 -0600, Tilghman Lesher wrote:
> I'd like to change the coding guidelines that the
braces are no longer
> optional

You've got a thumbs up from me!

-- 
Jarenessto
Community Relations Manager
Digium, Inc.


_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Jon Pounder wrote:

> I am not an active asterisk developer but I think
requiring that is a  
> great idea - I have used that style in c and other
languages for over  
> 20yrs for exactly the same reasons.

+1 here too

-- 
Kevin P. Fleming
Director of Software Technologies
Digium, Inc. - "The Genuine Asterisk Experience"
(TM)

_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

 +1

Tilghman Lesher wrote:
> I'd like to propose a change to the coding guidelines,
and I'm airing it here
> for specific feedback before we make this change. 
Currently, our standard
> is that if you have a single statement following an
if/for/while construct,
> then we do not require parentheses around the single
statement.  While this
> is legal C, it has caused a number of issues in the
past, where another
> statement was added at that indentation level, but
without adding the
> necessary braces.
>
> I'd like to change the coding guidelines that the
braces are no longer
> optional; that they are mandatory, and if this change
is approved, a janitor
> project will be posted shortly to convert legacy code
to the new standard.
>
> Are there any comments or objections before we
implement this change?
>
>   


-- 
--
Bird's The Word Technologies, Inc.
http://www.btwtech.com/




_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

On Tue, 26 Feb 2008, Tilghman Lesher wrote:

> I'd like to propose a change to the coding guidelines,
and I'm airing it here
> for specific feedback before we make this change. 
Currently, our standard
> is that if you have a single statement following an
if/for/while construct,
> then we do not require parentheses around the single
statement.  While this
> is legal C, it has caused a number of issues in the
past, where another
> statement was added at that indentation level, but
without adding the
> necessary braces.
>
> I'd like to change the coding guidelines that the
braces are no longer
> optional; that they are mandatory, and if this change
is approved, a janitor
> project will be posted shortly to convert legacy code
to the new standard.
>
> Are there any comments or objections before we
implement this change?

+1 -- should have been done from the start 

If the coding guidelines are up for review, how about
allowing // for 
commenting?

Thanks in advance,
------------------------------------------------------------
------------
Steve Edwards      sedwardssedwards.com      Voice:
+1-760-468-3867 PST
Newline                                             Fax:
+1-760-731-3000

_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

+2

BJ Weschke wrote:
>  +1
>
> Tilghman Lesher wrote:
>   
>> I'd like to propose a change to the coding
guidelines, and I'm airing it here
>> for specific feedback before we make this change. 
Currently, our standard
>> is that if you have a single statement following an
if/for/while construct,
>> then we do not require parentheses around the
single statement.  While this
>> is legal C, it has caused a number of issues in the
past, where another
>> statement was added at that indentation level, but
without adding the
>> necessary braces.
>>
>> I'd like to change the coding guidelines that the
braces are no longer
>> optional; that they are mandatory, and if this
change is approved, a janitor
>> project will be posted shortly to convert legacy
code to the new standard.
>>
>> Are there any comments or objections before we
implement this change?
>>
>>   
>>     
>
>
>   


_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Tilghman Lesher wrote:
> I'd like to propose a change to the coding guidelines,
and I'm airing it here
> for specific feedback before we make this change. 
Currently, our standard
> is that if you have a single statement following an
if/for/while construct,
> then we do not require parentheses around the single
statement.  While this
> is legal C, it has caused a number of issues in the
past, where another
> statement was added at that indentation level, but
without adding the
> necessary braces.
> 
> I'd like to change the coding guidelines that the
braces are no longer
> optional; that they are mandatory, and if this change
is approved, a janitor
> project will be posted shortly to convert legacy code
to the new standard.
> 
> Are there any comments or objections before we
implement this change?

While I like the idea, however disagree with "a janitor
project to 
convert all code" extension strongly. It will introduce
significant 
superfluous changes to the code, unnecessarily complicating
life of 
people who maintain custom patches for asterisk.

Regards,
-- 
Maksym Sobolyev
Sippy Software, Inc.
Internet Telephony (VoIP) Experts
T/F: +1-646-651-1110
Web: http://www.sippysoft.com


_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

On Tue, Feb 26, 2008 at 11:46:37AM -0600, Tilghman Lesher
wrote:
> I'd like to propose a change to the coding guidelines,
and I'm airing it here
> for specific feedback before we make this change. 
Currently, our standard
> is that if you have a single statement following an
if/for/while construct,
> then we do not require parentheses around the single
statement.  While this
> is legal C, it has caused a number of issues in the
past, where another
> statement was added at that indentation level, but
without adding the
> necessary braces.
> 
> I'd like to change the coding guidelines that the
braces are no longer
> optional; that they are mandatory, and if this change
is approved, a janitor
> project will be posted shortly to convert legacy code
to the new standard.

Sounds like a good idea. How good is indent in that task?

> 
> Are there any comments or objections before we
implement this change?

Could you please point to one or two such cases?

Current coding guidelines already require that a nested
block will have
braces. I wonder if those issues are related to bad
indentation.

-- 
               Tzafrir Cohen
icq#16849755              jabber:tzafrir.cohenxorcom.com
+972-50-7952406           mailto:tzafrir.cohenxorcom.com
http://www.xorcom.com 
iax:guestlocal.xorcom.com/tzafrir

_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Tilghman Lesher ?????:
> I'd like to propose a change to the coding guidelines,
and I'm airing it here
> for specific feedback before we make this change. 
Currently, our standard
> is that if you have a single statement following an
if/for/while construct,
> then we do not require parentheses around the single
statement.  While this
> is legal C, it has caused a number of issues in the
past, where another
> statement was added at that indentation level, but
without adding the
> necessary braces.
>
> I'd like to change the coding guidelines that the
braces are no longer
> optional; that they are mandatory, 
+1


> and if this change is approved, a janitor
> project will be posted shortly to convert legacy code
to the new standard.
>   
-1

Too complex task, a lot of small changes - the great chance
to add a new 
bugs. I think we shouldn't change existing (working) code
intentionally. 
We should convert legacy code "as we go" - with
other changes

> Are there any comments or objections before we
implement this change?
>
>   


_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Tzafrir Cohen пишет:
> On Tue, Feb 26, 2008 at 11:46:37AM -0600, Tilghman
Lesher wrote:
>   
>> I'd like to propose a change to the coding
guidelines, and I'm airing it here
>> for specific feedback before we make this change. 
Currently, our standard
>> is that if you have a single statement following an
if/for/while construct,
>> then we do not require parentheses around the
single statement.  While this
>> is legal C, it has caused a number of issues in the
past, where another
>> statement was added at that indentation level, but
without adding the
>> necessary braces.
>>
>> I'd like to change the coding guidelines that the
braces are no longer
>> optional; that they are mandatory, and if this
change is approved, a janitor
>> project will be posted shortly to convert legacy
code to the new standard.
>>     
>
> Sounds like a good idea. How good is indent in that
task?
>
>   
>> Are there any comments or objections before we
implement this change?
>>     
>
> Could you please point to one or two such cases?
>
> Current coding guidelines already require that a nested
block will have
> braces. I wonder if those issues are related to bad
indentation.
>
>   

micron:/usr/src/asterisk-1.4 # grep if */*.c | grep -v
"{"

apps/app_dial.c

435 if (option_verbose > 2)
436 ast_verbose( VERBOSE_PREFIX_2 "Everyone is
busy/congested at this 
time (%d:%d/%d/%d)n", numlines, numbusy,
numcongestion, numnochan);


_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

On Tue, Feb 26, 2008 at 11:06:31PM +0300, Sergey Tamkovich
wrote:
> Tzafrir Cohen пишет:
> > On Tue, Feb 26, 2008 at 11:46:37AM -0600, Tilghman
Lesher wrote:
> >   

> >> While this
> >> is legal C, it has caused a number of issues
in the past, where another
> >> statement was added at that indentation level,
but without adding the
> >> necessary braces.
> >
> > Could you please point to one or two such cases?
> >
> > Current coding guidelines already require that a
nested block will have
> > braces. I wonder if those issues are related to
bad indentation.
> >
> micron:/usr/src/asterisk-1.4 # grep if */*.c | grep -v
"{"
> 
> apps/app_dial.c
> 
> 435 if (option_verbose > 2)
> 436 ast_verbose( VERBOSE_PREFIX_2 "Everyone is
busy/congested at this 
> time (%d:%d/%d/%d)n", numlines, numbusy,
numcongestion, numnochan);

So you're saying that there's bad indentation here?

Bad indentation is something that should be fixed
regardless.

Is the following code clear to you?

#ifdef WE_CAN_DO_FOO
if (!foo->bar) {
#endif
	a()
	b()
	c()
#ifdef WE_CAN_DO_FOO
}
#endif

-- 
               Tzafrir Cohen
icq#16849755              jabber:tzafrir.cohenxorcom.com
+972-50-7952406           mailto:tzafrir.cohenxorcom.com
http://www.xorcom.com 
iax:guestlocal.xorcom.com/tzafrir

_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Sergey Tamkovich wrote:
> Tilghman Lesher ?????:
>> I'd like to propose a change to the coding
guidelines, and I'm airing it here
>> for specific feedback before we make this change. 
Currently, our standard
>> is that if you have a single statement following an
if/for/while construct,
>> then we do not require parentheses around the
single statement.  While this
>> is legal C, it has caused a number of issues in the
past, where another
>> statement was added at that indentation level, but
without adding the
>> necessary braces.
>>
>> I'd like to change the coding guidelines that the
braces are no longer
>> optional; that they are mandatory, 
> +1

+1 ... sort of.  I'd rather it just be
"encouraged".

>> and if this change is approved, a janitor
>> project will be posted shortly to convert legacy
code to the new standard.
>>   
> -1
> 
> Too complex task, a lot of small changes - the great
chance to add a new 
> bugs. I think we shouldn't change existing (working)
code intentionally. 
> We should convert legacy code "as we go" -
with other changes

Make that -2.  I think it's a bunch of pointless small
changes.  I don't mind
encouraging them in the future, but going back and changing
the existing code is
pointless IMO.

-- 
Russell Bryant
Senior Software Engineer
Open Source Team Lead
Digium, Inc.

_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Quoting Donny Kavanagh <donnykgmail.com>:

> I agree with russ and some others, we should make that
the policy for all
> new submitted code, but i dont think we should go back
and modify every
> single if statement without brackets, that would be a
nightmare.  And the
> possibility of introducing bugs would be large.

and so would the possibility of finding bugs that are there
already  
but undiscovered.

>
> On Tue, Feb 26, 2008 at 3:40 PM, Russell Bryant
<russelldigium.com> wrote:
>
>> Sergey Tamkovich wrote:
>> > Tilghman Lesher ?????:
>> >> I'd like to propose a change to the coding
guidelines, and I'm airing
>> it here
>> >> for specific feedback before we make this
change.  Currently, our
>> standard
>> >> is that if you have a single statement
following an if/for/while
>> construct,
>> >> then we do not require parentheses around
the single statement.  While
>> this
>> >> is legal C, it has caused a number of
issues in the past, where another
>> >> statement was added at that indentation
level, but without adding the
>> >> necessary braces.
>> >>
>> >> I'd like to change the coding guidelines
that the braces are no longer
>> >> optional; that they are mandatory,
>> > +1
>>
>> +1 ... sort of.  I'd rather it just be
"encouraged".
>>
>> >> and if this change is approved, a janitor
>> >> project will be posted shortly to convert
legacy code to the new
>> standard.
>> >>
>> > -1
>> >
>> > Too complex task, a lot of small changes - the
great chance to add a new
>> > bugs. I think we shouldn't change existing
(working) code intentionally.
>> > We should convert legacy code "as we
go" - with other changes
>>
>> Make that -2.  I think it's a bunch of pointless
small changes.  I don't
>> mind
>> encouraging them in the future, but going back and
changing the existing
>> code is
>> pointless IMO.
>>
>> --
>> Russell Bryant
>> Senior Software Engineer
>> Open Source Team Lead
>> Digium, Inc.
>>
>> _______________________________________________
>> --Bandwidth and Colocation Provided by http://www.api-digital.c
om--
>>
>> asterisk-dev mailing list
>> To UNSUBSCRIBE or update options visit:
>>   http://lists.digium.com/mailman/listinfo/asterisk-dev
>>
>



Jon Pounder

    _/_/_/  _/    _/  _/       _/_/_/  _/    _/  _/_/_/_/
     _/    _/_/  _/  _/         _/    _/_/  _/  _/_/
    _/    _/  _/_/  _/         _/    _/  _/_/  _/
_/_/_/  _/    _/  _/_/_/_/ _/_/_/  _/    _/  _/_/_/_/


Inline Internet Systems Inc.
Thorold, Ontario, Canada

Tools to Power Your e-Business Solutions
www.inline.net
www.ihtml.com
www.ihtmlmerchant.com
www.opayc.com



_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

On 26/02/2008, at 18:20, Tzafrir Cohen wrote:
>> micron:/usr/src/asterisk-1.4 # grep if */*.c | grep
-v "{"
>>
> So you're saying that there's bad indentation here?

I guess this is what Sergey meant, perhaps:

$ grep -rn if */*.c | grep -v '{' | wc -l
    21422 (matches)

$ grep -r if */*.c | grep -v '{' | sed 's/:.*$//' | sort |
wc -l
      283 (files)

Too many places in too many files to work on, if the
command's result  
is accurate. It's like a huge blinking billboard with
"if I were you  
I wouldn't dare" written on it.

--
Caio Begotti <http://caio.ueberalles
.net>


_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

On Tue, Feb 26, 2008 at 3:40 PM, Russell Bryant
<russelldigium.com> wrote:
>  >> I'd like to change the coding guidelines that
the braces are no longer
>  >> optional; that they are mandatory,
>  > +1
>
>  +1 ... sort of.  I'd rather it just be
"encouraged".

This is obviously a good idea. I just wanted to comment on
the two
philosophies proposed so far -- "mandatory" vs.
"encouraged". I think
if we just say that this is "encouraged" then it
is no different from
what we have today. Most developers already know that this
is always a
good idea. I've found myself deleting curly braces around
single line
statements to make my patch look-and-feel like existing
code. I'd say,
make it "mandatory".

-- 
Raj Jain

mailto:rj2807 at gmail dot com
sip:rjain at iptel dot org

_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

BJ Weschke wrote:
>  +1
>
> Tilghman Lesher wrote:
>   
>> I'd like to propose a change to the coding
guidelines, and I'm airing it here
>> for specific feedback before we make this change. 
Currently, our standard
>> is that if you have a single statement following an
if/for/while construct,
>> then we do not require parentheses around the
single statement.  While this
>> is legal C, it has caused a number of issues in the
past, where another
>> statement was added at that indentation level, but
without adding the
>> necessary braces.
>>
>> I'd like to change the coding guidelines that the
braces are no longer
>> optional; that they are mandatory, and if this
change is approved, a janitor
>> project will be posted shortly to convert legacy
code to the new standard.
>>
>> Are there any comments or objections before we
implement this change?
>>
>>   
>>     
>
>
>   
What about coming up with a standard 'indent'
 command that will format the code to Digium's requirements
?
Add this as a rule in the Makefile

Each developer can then code in whatever style he likes. 
When finished 
and before making patches execute 'make indent' and you are
done.


Paul

-- 
Technical Director, GCCS
Tel: 086 111 3433 Fax: 086 111 3520 Cel: 076 072 7906
web: http://www.gccs.co.za


_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

To repeat what I wrote:

On Tue, Feb 26, 2008 at 10:30:02PM +0200, Tzafrir Cohen
wrote:
> On Tue, Feb 26, 2008 at 11:46:37AM -0600, Tilghman
Lesher wrote:
> > I'd like to propose a change to the coding
guidelines, and I'm airing it here
> > for specific feedback before we make this change. 
Currently, our standard
> > is that if you have a single statement following
an if/for/while construct,
> > then we do not require parentheses around the
single statement.  While this
> > is legal C, it has caused a number of issues in
the past, where another
> > statement was added at that indentation level, but
without adding the
> > necessary braces.
> >
> > Are there any comments or objections before we
implement this change?
> 
> Current coding guidelines already require that a nested
block will have
> braces. I wonder if those issues are related to bad
indentation.
> 
> Could you please point to one or two such cases?

Again, I'm not convinced. And nobody seems to bother
providing
examples.

With the lengthy functions that currently exist in Asterisk,
I rather
spare some lines. It looks simpler to read.

-- 
               Tzafrir Cohen
icq#16849755              jabber:tzafrir.cohenxorcom.com
+972-50-7952406           mailto:tzafrir.cohenxorcom.com
http://www.xorcom.com 
iax:guestlocal.xorcom.com/tzafrir

_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

On Wednesday 27 February 2008 01:26:41 Paul Hewlett wrote:
> BJ Weschke wrote:
> >  +1
> >
> > Tilghman Lesher wrote:
> >> I'd like to propose a change to the coding
guidelines, and I'm airing it
> >> here for specific feedback before we make this
change.  Currently, our
> >> standard is that if you have a single
statement following an
> >> if/for/while construct, then we do not require
parentheses around the
> >> single statement.  While this is legal C, it
has caused a number of
> >> issues in the past, where another statement
was added at that
> >> indentation level, but without adding the
necessary braces.
> >>
> >> I'd like to change the coding guidelines that
the braces are no longer
> >> optional; that they are mandatory, and if this
change is approved, a
> >> janitor project will be posted shortly to
convert legacy code to the new
> >> standard.
> >>
> >> Are there any comments or objections before we
implement this change?
>
> What about coming up with a standard 'indent'
>  command that will format the code to Digium's
requirements ?
> Add this as a rule in the Makefile
>
> Each developer can then code in whatever style he
likes.  When finished
> and before making patches execute 'make indent' and you
are done.

I think the problem with that is that our coding style is
very similar to a
particular set of indent arguments, as described in the
coding guidelines,
but there is no set of indent arguments that is EXACTLY our
coding guidelines,
and anyway, I don't think indent will insert missing braces,
missing
documentation, fix macro misuse, or various other problems
that need to be
corrected.

-- 
Tilghman

_______________________________________________
--Bandwidth and Colocation Provided by http://www.api-digital.c
om--

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev