[asterisk-dev] [Code Review] avoid many cppcheck (#2)

[wdoekes]

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



trunk/channels/chan_alsa.c
<https://reviewboard.asterisk.org/r/1743/#comment10404>

    Red blob



trunk/channels/chan_h323.c
<https://reviewboard.asterisk.org/r/1743/#comment10405>

    Add braces since you're touching this line.



trunk/channels/chan_unistim.c
<https://reviewboard.asterisk.org/r/1743/#comment10406>

    Wrong: OpenHistory takes care of most of the fclose's.
    
    (1) Change OpenHistory to set *f = NULL after fclose and here do an if(f){fclose(f);}
    
    or
    
    (2) Have OpenHistory fclose(*f) if count==0.
    
    Logically, I'd prefer the first. But the second will reduce the amount of extra if's you'll have to write. Better go with the last one.



trunk/channels/chan_unistim.c
<https://reviewboard.asterisk.org/r/1743/#comment10407>

    If you chose option (2), then undo this change.



trunk/channels/chan_unistim.c
<https://reviewboard.asterisk.org/r/1743/#comment10408>

    If you chose option (2), then undo this change.



trunk/channels/chan_unistim.c
<https://reviewboard.asterisk.org/r/1743/#comment10409>

    > if channel_to_session(ast) returns NULL, without that new if, we'll get a segfault.
    
    Of course, but that has not occurred (often), otherwise this would have been fixed by now.
    
    But you're right, I don't see why some functions should skip the check while others do it:
    
    please add same check to beginnning of unistim_sendtext.



trunk/codecs/codec_resample.c
<https://reviewboard.asterisk.org/r/1743/#comment10410>

    Add a comment that this works only because unload_module's calls to ast_unregister_translator won't fail for the translators that failed to register.
    
    unload_module already relies on that btw, since it also tries to unload the x==y translators that were never registered.
    
    All the more reason to break at the first sign of failure and not continue on when we're going to undo everything anyway.



trunk/funcs/func_env.c
<https://reviewboard.asterisk.org/r/1743/#comment10411>

    How about:
    
    pos = (end < sizeof(fbuf) ? fbuf + end - 1 : fbuf + sizeof(fbuf) - 1)



trunk/res/res_jabber.c
<https://reviewboard.asterisk.org/r/1743/#comment10412>

    Your cppcheck probably has !defined(HAVE_OPENSSL). With it, the break is still doing something.
    
    Move the break to before the #endif.



trunk/res/res_musiconhold.c
<https://reviewboard.asterisk.org/r/1743/#comment10413>

    Move this one up a line, it belongs with the ast_free, not with the getcwd.


- wdoekes


On March 1, 2012, 12:22 a.m., junky wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1743/
> -----------------------------------------------------------
> 
> (Updated March 1, 2012, 12:22 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch is to prevent many more cppcheck warnings.
> 
> 
> Diffs
> -----
> 
>   trunk/apps/app_minivm.c 357660 
>   trunk/apps/app_osplookup.c 357660 
>   trunk/cdr/cdr_pgsql.c 357660 
>   trunk/cdr/cdr_sqlite3_custom.c 357660 
>   trunk/channels/chan_alsa.c 357660 
>   trunk/channels/chan_dahdi.c 357660 
>   trunk/channels/chan_gtalk.c 357660 
>   trunk/channels/chan_h323.c 357660 
>   trunk/channels/chan_unistim.c 357660 
>   trunk/channels/vcodecs.c 357660 
>   trunk/codecs/codec_dahdi.c 357660 
>   trunk/codecs/codec_resample.c 357660 
>   trunk/formats/format_h264.c 357660 
>   trunk/funcs/func_devstate.c 357660 
>   trunk/funcs/func_env.c 357660 
>   trunk/main/ast_expr2.fl 357660 
>   trunk/main/ast_expr2f.c 357660 
>   trunk/main/asterisk.c 357660 
>   trunk/main/data.c 357660 
>   trunk/res/res_config_ldap.c 357660 
>   trunk/res/res_config_sqlite3.c 357660 
>   trunk/res/res_corosync.c 357660 
>   trunk/res/res_format_attr_celt.c 357660 
>   trunk/res/res_format_attr_silk.c 357660 
>   trunk/res/res_http_post.c 357660 
>   trunk/res/res_jabber.c 357660 
>   trunk/res/res_musiconhold.c 357660 
>   trunk/res/res_odbc.c 357660 
>   trunk/res/res_phoneprov.c 357660 
> 
> Diff: https://reviewboard.asterisk.org/r/1743/diff
> 
> 
> Testing
> -------
> 
> used cppcheck 1.52
> it compiles ;)
> 
> 
> Thanks,
> 
> junky
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20120301/47f4d29a/attachment-0001.htm>