> 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
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

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

Reply via email to