Summary:	ASTERISK-04674: [patch] [post 1.2] whitespace cleanup for chan_sip.c according to CODING GUIDELINES
Reporter:	Frank Sautter (xylome)	Labels:
Date Opened:	2005-07-25 09:46:52	Date Closed:	2011-06-07 14:10:28
Priority:	Minor	Regression?	No
Status:	Closed/Complete	Components:	Channels/chan_sip/General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) whitespace.chan_sip.r792.txt ( 1) whitespace.chan_sip.r796.txt ( 2) whitespace.chan_sip.r797.txt ( 3) whitespace.chan_sip.r797v2.txt ( 4) whitespace.chan_sip.r801.txt ( 5) whitespace.chan_sip.r801v2.txt ( 6) whitespace.chan_sip.r802.txt ( 7) whitespace.chan_sip.r806.txt
Description:	this is mainly a whitespace cleanup according to the CODING GUIDELINES * converted space indentions to tabs * aligned comments in the head of the file * corrected if(, while(, for(, switch( statements * corrected case indenting besides code cleanup: * some spelling errors * moved forward declarations from midst od code to the head section of the file * moved #defines from midst of code to the head section of the file * 4 new compressed sip abbreviations added
Comments:	By: Olle Johansson (oej) 2005-07-25 10:03:01 Seems like a good patch. Just to complain, the only thing I don't like is the long lines that you make here and there. Otherwise, it's a good cleaning up of the file. By: Olle Johansson (oej) 2005-07-25 10:03:24 Thank you! By: Olle Johansson (oej) 2005-07-25 10:04:37 There's some struc declarations in the middle of the code that I also would like to move up, like the RTP structure. Do you agree with that? By: Frank Sautter (xylome) 2005-07-25 10:20:55 oej: i agree with you, and i think there are some other 100 places in the code that have to be corrected, but we should not do this in this patch. i'd say... let's get  this commited to cvs soon. we can take care of other code cleanup with other patches. i don't want to create another monster patch. as you said once oej: keep patches small to get them digested fast thanks for your comments oej!!! By: Tilghman Lesher (tilghman) 2005-07-25 14:30:34 - The define of REG_STATE_NOAUTH still has tabs in it. - There's several other places where you seem to have left tabs in the middle of a line, whereas in most places, you've removed tabs in the middle of a line.  There should be some consistency, especially since this is a formatting patch. - Around line 1651, there's a set of text which is partially indented with tabs, but also partially indented with space.  Shouldn't this just be tabs? - Around line 2125 and other places, you've switched the end of a multiline comment from a separate line to the same as the previous line.  Speaking as a coder, it's much better to have this end-of-comment on a separate line, to allow for additional list items to be placed in the comment in the future. - Around line 2233, you've stripped the {} braces around an if statement, which is contrary to the coding guidelines.  It's good practice to ALWAYS use braces around any conditional or looped statement. It's a good effort, but it needs substantial improvement before it should be committed. By: Frank Sautter (xylome) 2005-07-25 15:00:50 updated once again to a moving target... Corydon76: thanks for your review! i changed most of the things you mentioned. the patch around line 1651 (now 1653) was intentional (and there should be some other indents like this). as this is a multiline function call first the line is indented like the function call itself with 'tabs'. then the indention is done with 'spaces' until it aligns with the opening braces. but keep in mind: this patch has NOT the attitude to resolve all formatting problems that exist in this file (i'm sure there are some other 1000 issues). this is just the output of my effort to 'white space clean up' this code. i know it's not perfect, but i thinks it's improving the current code layout. By: Frank Sautter (xylome) 2005-07-25 15:11:12 updated to revision 789 of chan_sip.c By: Tilghman Lesher (tilghman) 2005-07-25 17:15:54 - More inline tabs at around line 274, 2041. - Around 799, let's keep those spaces or perhaps even add spaces so the comments line up. - I still think the change at 1634 is a mistake.  Patches at lines 10514 and 10612 show you doing something different.  Again, consistency. - At the end of 4249, you've erroneously removed a tab.  You might want to add the braces to the previous if, since that makes it much more obvious. By: Frank Sautter (xylome) 2005-07-26 05:04:33 updated to revision 792 Corydon76: i implemented your suggestions. i hope i removed all remaining inline tabs. By: Tilghman Lesher (tilghman) 2005-07-26 11:15:14 Okay, I'm satisfied with this patch.  You may have to do one more patch, as several people have reported on the -dev list that something got broken in chan_sip yesterday, but hold off on the new patch until there's a confirmed fix in place. By: Tilghman Lesher (tilghman) 2005-07-26 23:03:04 Okay, fix has been submitted, so go ahead and update, and submit your patch one more time. By: Frank Sautter (xylome) 2005-07-29 11:12:37 updated to revision 797 By: Tilghman Lesher (tilghman) 2005-07-29 11:47:26 797: - DEBUG_READ and DEBUG_SEND have inline tabs. - Another inline tab near 8554.  Another at 10353. By: Frank Sautter (xylome) 2005-08-01 09:47:35 spent some more hours on cleaning up the code... now the patch is more than twice as big :-/ * removed trailing whitespace * used regexp to find inline tabs and some other nasty whitespaces (hope i got them all) * reformatted some multiline comments (within 80 columns; beginning with *) By: Frank Sautter (xylome) 2005-08-03 04:20:40 updated to revision 801 By: Frank Sautter (xylome) 2005-08-03 09:33:37 * did some more cleanup around ',', ';', '(', ')', '=' and '==' * moved the compressed sip aliases to another patch ASTERISK-4766 By: Mark Spencer (markster) 2005-08-03 14:06:03 Does not apply to latest CVS head.  Due to the nature of this patch, we probably need you to contact me as soon as it's ready so I can put it in right away. By: Frank Sautter (xylome) 2005-08-03 16:01:37 patch is updated to revision 802 of chan_sip.c By: Frank Sautter (xylome) 2005-08-09 13:38:00 patch is updated to revision 806 of chan_sip.c By: Kevin P. Fleming (kpfleming) 2005-08-22 21:22:01 Can we hold off on this one until after 1.2 is released? I don't want to break all the other pending patches for just whitespace only changes. By: Michael Jerris (mikej) 2005-08-22 21:27:03 why can't we just run the entire tree through indent after 1.2 and cleanup what's left over? By: Kevin P. Fleming (kpfleming) 2005-08-22 21:40:42 That may very well happen, as we are also considering switching to another revision control system and that would be an opportune time for such mass changes. By: Kevin P. Fleming (kpfleming) 2005-09-28 22:54:22 Can we pull the non-whitespace changes out of this and get them merged now, and leave the other changes for after we've run this through some tool to take care of most of it? By: Russell Bryant (russell) 2005-11-23 11:49:30.000-0600 Feel free to re-open this bug if you have a patch for the changes that are not whitespace changes.  Thanks!