[asterisk-dev] [Code Review] 4554: clang compiler warning: -Wunused-value

[rmudgett]

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


I dislike this warning.  It is more nuisance than helpful.  I once worked with compiler that had a similar warning.  The lengths you sometimes have to go to eradicate the the warning can be extream.

There are several places in this patch that change code behavior and introduce bugs.

- rmudgett


On March 28, 2015, 7:44 p.m., Diederik de Groot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4554/
> -----------------------------------------------------------
> 
> (Updated March 28, 2015, 7:44 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
> 
> Changes:
> apps/app_agent_pool.c
> multiple occasions where return value from ast_channel_ref and ast_channel_unref is not checked not used.
> return value from AST_LIST_REMOVE should be checked.
> Possible issues with leaked logged reference (added remark)
> 
> channels/chan_iax2.c
> return values from ast_callid_ref and ast_callid_unref can/should be used to update the pointer
> return value from AST_SCHED_DEL should be used to update the scheduled variable
> return value from AST_LIST_REMOVE should be checked.
> 
> channels/iax2/parser.c
> return value from AST_LIST_REMOVE should be checked.
> 
> include/asterisk/stringfields.h
> Quick fix to send the returned value from ast_string_field_ptr_set into the void.
> 
> Work in Progress:
> There are too many sources issues with "-Wunused-value" for one person to create all the fixes. If we want to actually use this warning we will need a couple of extra hands-on.
> 
> Should we suppress "-Wunused-value" ? :
> I think some of the issues pointed out by clang could be very helpfull, for example the "unref/ref/AST_LIST_REMOVE" changes that where needed in chan_iax2.c and apps/app_agent_pool.c. Some others a less interesting, for example the SCHED_DEL fixes.
> 
> 
> Diffs
> -----
> 
>   /branches/13/include/asterisk/stringfields.h 433444 
>   /branches/13/channels/iax2/parser.c 433444 
>   /branches/13/channels/chan_iax2.c 433444 
>   /branches/13/apps/app_agent_pool.c 433444 
> 
> Diff: https://reviewboard.asterisk.org/r/4554/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> ASTCFLAGS="-Wno-error=unused-value" make &| grep "[-W" -B1 -A2
>   https://reviewboard.asterisk.org/media/uploaded/files/2015/03/28/83f2f382-9ef2-41cf-8e7a-c188c014c17e__make.report
> 
> 
> Thanks,
> 
> Diederik de Groot
> 
>

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