[asterisk-dev] [Code Review] 4535: clang compiler warning: -Wenum-conversion

[Diederik de Groot]

> On March 26, 2015, 11:57 p.m., Diederik de Groot wrote:
> > /branches/13/include/asterisk/strings.h, lines 1236-1237
> > <https://reviewboard.asterisk.org/r/4535/diff/1/?file=72977#file72977line1236>
> >
> >     Not 100% sure if this substition is correct. Please verify.
> 
> Matt Jordan wrote:
>     Well, it's "correct", but odd.
>     
>     Most container allocation routines take in two parameters - the container options, and the ao2 allocation options. This is taking in the ao2_container_opts, but treating it as if it were the ao2 allocation options. Two factors make me think the code change you have here is correct, and that we should not bother exposing ao2_container_opts:
>     (1) Most container options deal with how things are stored internally and traversal mechanisms. For a simple bucket of strings, those options aren't generally useful.
>     (2) The usage of the function involves attempting to create a container of strings without a lock (AO2_ALLOC_OPT_LOCK_NOLOCK), which is an ao2 allocation option.
>     
>     So I think this is correct - the first parameter should be of type enum ao2_alloc_opts.

I was a little worried about doing something wrong here. Casting enums to int to make them pass is always a sign of problem ahead. Good that clang is a little more vigilant about these.


- Diederik


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


On March 28, 2015, 4:27 p.m., Diederik de Groot wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4535/
> -----------------------------------------------------------
> 
> (Updated March 28, 2015, 4:27 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:-Wenum-conversion
> 
> Changes:
> /branches/13/channels/chan_pjsip.c:923
> Wrong enum being used
> 
> /branches/13/channels/chan_sip.c:19102
> Incorrect enum
> 
> /branches/13/channels/chan_sip.c:19107
> Incorrect enum returned
> 
> /branches/13/include/asterisk/strings.h:1236
> Replaced enum ao2_container_opts opts -> enum ao2_alloc_opts opts
> Not 100% sure if this substition is correct. Please verify.
> 
> /branches/13/main/strings.c:173
> Replaced enum ao2_container_opts opts -> enum ao2_alloc_opts opts
> Not 100% sure if this substition is correct. Please verify.
> 
> /branches/13/res/res_stasis.c:1803 
> Incorrect enum for return value
> 
> 
> Diffs
> -----
> 
>   /branches/13/res/res_stasis.c 433444 
>   /branches/13/main/strings.c 433444 
>   /branches/13/include/asterisk/strings.h 433444 
>   /branches/13/channels/chan_sip.c 433444 
>   /branches/13/channels/chan_pjsip.c 433444 
> 
> Diff: https://reviewboard.asterisk.org/r/4535/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Diederik de Groot
> 
>

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