[asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable

[Diederik de Groot]

> On March 26, 2015, 6:37 p.m., Diederik de Groot wrote:
> > /branches/13/funcs/func_env.c, line 728
> > <https://reviewboard.asterisk.org/r/4526/diff/1/?file=72920#file72920line728>
> >
> >     Note the double closing parens after the first sizeof, i actually finishes the ast_str_append_substr command and then continues with the second sizeof.
> >     
> >     Not a 100% sure if this is corrected in the right way, but the original doesn't look right either.
> >
> 
> Matt Jordan wrote:
>     I'd double check that the unit tests still pass with the change.
> 
> Diederik de Groot wrote:
>     Currently not running any tests, just making the changes for now. Sounds silly, but it is enough work as it is. Automated testing should spit out any issues later on i hope.
> 
> Matt Jordan wrote:
>     Well, that would mean someone else would have to go fix it. :-P
>     
>     It's really easy to run a single unit test to double check that the logic still works - particularly when there's already a unit test written for a particular module. To do that, you:
>     
>     * Configure Asterisk to enable dev mode: ./configure --enable-dev-mode
>     * Enable TEST_FRAMEWORK in menuselect's build options
>     * On the CLI, execute 'test execute category /funcs/func_env/'
>     
>     Granted, since Asterisk won't compile in dev mode with clang (as WARNINGs are promoted to ERRORs), you'll need to double check it compiled with gcc, but for things like this where stuff is questionable, it doesn't hurt to make sure that the existing tests pass locally before things get committed.

Now that i am finished i can run a couple of these, i guess. Will give it a go, after my CPU cools down a bit ;-P


- Diederik


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


On March 27, 2015, 12:09 p.m., Diederik de Groot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4526/
> -----------------------------------------------------------
> 
> (Updated March 27, 2015, 12:09 p.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
> 
> clang compiler warning:-Wunused-value -Wunused-variable -Wunused-const-variable
> 
> Changes:
> apps/app_queue.c: removed unused qpm_cmd_usage[], qum_cmd_usage[], qsmp_cmd_usage[]
> cel/cel_sqlite3_custom.c: removed unused name[] = "cel_sqlite3_custom"
> channels/chan_pjsip.c: removed unused desc[] = "PJSIP Channel"
> codecs/gsm/src/gsm_create.c: removed unused ident[] = "$Header$"
> funcs/func_env.c:729: Fixed ast_str_append_substr. Needs to be reviewed by code owner.
> main/config.c: inline functions have to be static for clang to grok them
> main/editline/np/strlcat.c: removed unused rcsid = "$OpenBSD: strlcat.c,v 1.2 1999/06/17 16:28:58 millert Exp $"
> main/editline/np/strlcpy.c: removed unused rcsid = "$OpenBSD: strlcpy.c,v 1.4 1999/05/01 18:56:41 millert Exp $"
> main/features.c: removed unused channel_app_data_datastore struct
> main/security_events.c: removed unused TIMESTAMP_STR_LEN
> utils/conf2ael.c: removed unused cfextension_states
> utils/extconf.c: removed unused cfextension_states
> 
> 
> Diffs
> -----
> 
>   /branches/13/utils/extconf.c 433444 
>   /branches/13/utils/conf2ael.c 433444 
>   /branches/13/main/security_events.c 433444 
>   /branches/13/main/features.c 433444 
>   /branches/13/main/editline/np/strlcpy.c 433444 
>   /branches/13/main/editline/np/strlcat.c 433444 
>   /branches/13/main/config.c 433470 
>   /branches/13/funcs/func_env.c 433444 
>   /branches/13/codecs/gsm/src/gsm_create.c 433444 
>   /branches/13/channels/chan_pjsip.c 433444 
>   /branches/13/cel/cel_sqlite3_custom.c 433444 
>   /branches/13/apps/app_queue.c 433444 
> 
> Diff: https://reviewboard.asterisk.org/r/4526/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Diederik de Groot
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20150327/4dd6424c/attachment.html>