[asterisk-dev] [Code Review] 4529: clang compiler warning: -Wno-sometimes-uninitialized

[Matt Jordan]

> On March 27, 2015, 8:23 a.m., Matt Jordan wrote:
> > /branches/13/pbx/pbx_config.c, lines 960-961
> > <https://reviewboard.asterisk.org/r/4529/diff/2/?file=72981#file72981line960>
> >
> >     Well, this won't quite work, as appdata can be NULL. strlen isn't NULL safe.
> >     
> >     As such, you'll need to do something like:
> >     
> >     int escaped_len = (!ast_strlen_zero(appdata) ? 2 * strlen(appdata) : 0) + 1;
> >     
> >
> 
> Diederik de Groot wrote:
>     Thanks for the pointer ! No need to count the string length twice, just a null pointer check should suffice. Also need to set the escaped_len to 1 if appdata==NULL, so that it will be able to contain '\0'.
> 
> rmudgett wrote:
>     ast_strlen_zero() is a boolean test.  It does not actually do a strlen.  It is just: (!str || (*str == '\0')).
> 
> Diederik de Groot wrote:
>     Ahh ok i didn't know that. It does have the same effect to my version though (in this particular case: even if appdata[0]='\0' it would still result in escaped_len=1). If you guys prefer the ast_strlen_zero test instead, i will update it though.

Since the Asterisk project is over 15 years old and has had a _lot_ of contributors, we care a lot about making sure that the code uses the same techniques and approaches throughout the code base. Otherwise, anyone who wants to contribute a patch would have to deal with numerous different conflicting "styles."

Granted, we still deal with this sometimes, as we didn't start caring that much about this until it started to bite us, but we definitely care now!


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/4529/#review14894
-----------------------------------------------------------


On March 27, 2015, 11:32 a.m., Diederik de Groot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4529/
> -----------------------------------------------------------
> 
> (Updated March 27, 2015, 11:32 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24917
>     https://issues.asterisk.org/jira/browse/ASTERISK-24917
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> clang's static analyzer will throw quite a number warnings / errors during compilation, some of which can be very helpfull in finding corner-case bugs\nclang compiler warning:-Wno-sometimes-uninitialized
> 
> 
> Diffs
> -----
> 
>   /branches/13/pbx/pbx_config.c 433444 
> 
> Diff: https://reviewboard.asterisk.org/r/4529/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Diederik de Groot
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150328/10592ab0/attachment-0001.html>