Summary:	ASTERISK-27793: cppcheck identifies redundant "if"
Reporter:	Ilya Shipitsin (chipitsine)	Labels:	patch pjsip
Date Opened:	2018-04-06 11:21:45	Date Closed:	2018-04-24 18:41:05
Priority:	Trivial	Regression?	No
Status:	Closed/Complete	Components:	Addons/chan_ooh323
Versions:	15.3.0	Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 0001-clean-cppcheck-warning.patch ( 1) ASTERISK-27793.patch
Description:	there's already if(dfr), no need for one more
Comments:	By: Asterisk Team (asteriskteam) 2018-04-06 11:21:46.488-0500 Thanks for creating a report! The issue has entered the triage process. That means the issue will wait in this status until a Bug Marshal has an opportunity to review the issue. Once the issue has been reviewed you will receive comments regarding the next steps towards resolution. A good first step is for you to review the [Asterisk Issue Guidelines|https://wiki.asterisk.org/wiki/display/AST/Asterisk+Issue+Guidelines] if you haven't already. The guidelines detail what is expected from an Asterisk issue report. Then, if you are submitting a patch, please review the [Patch Contribution Process|https://wiki.asterisk.org/wiki/display/AST/Patch+Contribution+Process]. By: Alexander Anikin (may213) 2018-04-09 17:19:53.206-0500 Hi, Please specify which compiler you use. Tested of debian 9/gcc 6.3 and haven't such warning By: Ilya Shipitsin (chipitsine) 2018-04-10 00:10:34.762-0500 it's cppcheck ( https://github.com/danmar/cppcheck ) cd asterisk script cppcheck --force --enable=all . ... ... Ctrl-D grep '(warning' typescript | grep 'Either the condition' {noformat} [addons/ooh323c/src/memheap.c:1067] -> [addons/ooh323c/src/memheap.c:1065]: (warning) Either the condition 'ppMemHeap!=0' is redundant or there is possible null pointer dereference: ppMemHeap. [addons/ooh323c/src/ooCalls.c:806] -> [addons/ooh323c/src/ooCalls.c:808]: (warning) Either the condition '!call' is redundant or there is possible null pointer dereference: call. [addons/ooh323c/src/ooCapability.c:58] -> [addons/ooh323c/src/ooCapability.c:66]: (warning) Either the condition '!call' is redundant or there is possible null pointer dereference: call. [addons/ooh323c/src/ooCapability.c:628] -> [addons/ooh323c/src/ooCapability.c:627]: (warning) Either the condition '!params' is redundant or there is possible null pointer dereference: params. [addons/ooh323c/src/ooCapability.c:813] -> [addons/ooh323c/src/ooCapability.c:812]: (warning) Either the condition '!events' is redundant or there is possible null pointer dereference: events. [addons/ooh323c/src/ooGkClient.c:2339] -> [addons/ooh323c/src/ooGkClient.c:2337]: (warning) Either the condition '!perCallInfo' is redundant or there is possible null pointer dereference: perCallInfo. [addons/ooh323c/src/ooh245.c:360] -> [addons/ooh323c/src/ooh245.c:359]: (warning) Either the condition 'request==NULL' is redundant or there is possible null pointer dereference: request. [addons/ooh323c/src/ooq931.c:2444] -> [addons/ooh323c/src/ooq931.c:2443]: (warning) Either the condition '!pNewAlias' is redundant or there is possible null pointer dereference: pNewAlias. [addons/res_config_mysql.c:505] -> [addons/res_config_mysql.c:466]: (warning) Either the condition 'if(initfield)' is redundant or there is possible null pointer dereference: initfield. [apps/app_minivm.c:1262] -> [apps/app_minivm.c:1264]: (warning) Either the condition 'vmu' is redundant or there is possible null pointer dereference: vmu. [apps/app_minivm.c:1262] -> [apps/app_minivm.c:1265]: (warning) Either the condition 'vmu' is redundant or there is possible null pointer dereference: vmu. [apps/app_minivm.c:2190] -> [apps/app_minivm.c:2194]: (warning) Either the condition 'duration_string' is redundant or there is possible null pointer dereference: duration_string. [apps/app_minivm.c:3436] -> [apps/app_minivm.c:3440]: (warning) Either the condition 'countername' is redundant or there is possible null pointer dereference: countername. [channels/chan_mgcp.c:985] -> [channels/chan_mgcp.c:997]: (warning) Either the condition 'p' is redundant or there is possible null pointer dereference: p. [channels/chan_mgcp.c:2179] -> [channels/chan_mgcp.c:2176]: (warning) Either the condition '!sub' is redundant or there is possible null pointer dereference: sub. [channels/chan_mgcp.c:2659] -> [channels/chan_mgcp.c:2665]: (warning) Either the condition 'p' is redundant or there is possible null pointer dereference: p. [channels/chan_mgcp.c:2843] -> [channels/chan_mgcp.c:2841]: (warning) Either the condition 'sub?sub->id:-1' is redundant or there is possible null pointer dereference: sub. [channels/chan_mgcp.c:3474] -> [channels/chan_mgcp.c:3471]: (warning) Either the condition 'if(p)' is redundant or there is possible null pointer dereference: p. [channels/chan_misdn.c:6802] -> [channels/chan_misdn.c:6798]: (warning) Either the condition 'newbc?newbc->pid:-1' is redundant or there is possible null pointer dereference: newbc. [channels/chan_sip.c:13194] -> [channels/chan_sip.c:13208]: (warning) Either the condition 'max_packet_size' is redundant or there is possible null pointer dereference: max_packet_size. [channels/chan_skinny.c:7636] -> [channels/chan_skinny.c:7291]: (warning) Either the condition 'if(req)' is redundant or there is possible null pointer dereference: req. [channels/chan_unistim.c:5525] -> [channels/chan_unistim.c:5515]: (warning) Either the condition '!pte' is redundant or there is possible null pointer dereference: pte. [channels/chan_vpb.cc:2362] -> [channels/chan_vpb.cc:2364]: (warning) Either the condition 'fr' is redundant or there is possible null pointer dereference: fr. [channels/chan_vpb.cc:564] -> [channels/chan_vpb.cc:585]: (warning) Either the condition '!f' is redundant or there is possible null pointer dereference: f. [channels/misdn/isdn_lib.c:1936] -> [channels/misdn/isdn_lib.c:1934]: (warning) Either the condition 'if(hold_bc)' is redundant or there is possible null pointer dereference: hold_bc. [channels/misdn/isdn_lib.c:4536] -> [channels/misdn/isdn_lib.c:4535]: (warning) Either the condition '!stack' is redundant or there is possible null pointer dereference: stack. [codecs/lpc10/analys.c:457] -> [codecs/lpc10/analys.c:641]: (warning) Either the condition 'voice' is redundant or there is possible null pointer dereference: voice. [codecs/lpc10/analys.c:457] -> [codecs/lpc10/analys.c:642]: (warning) Either the condition 'voice' is redundant or there is possible null pointer dereference: voice. [codecs/lpc10/voicin.c:502] -> [codecs/lpc10/voicin.c:594]: (warning) Either the condition 'ivrc' is redundant or there is possible null pointer dereference: ivrc. [main/astfd.c:282] -> [main/astfd.c:283]: (warning) Either the condition '!ptr' is redundant or there is possible null pointer dereference: ptr. [main/astobj2.c:709] -> [main/astobj2.c:695]: (warning) Either the condition '(obj)==NULL' is redundant or there is possible null pointer dereference: obj. [main/astobj2.c:709] -> [main/astobj2.c:696]: (warning) Either the condition '(obj)==NULL' is redundant or there is possible null pointer dereference: obj. [main/astobj2.c:709] -> [main/astobj2.c:697]: (warning) Either the condition '(obj)==NULL' is redundant or there is possible null pointer dereference: obj. [main/astobj2.c:709] -> [main/astobj2.c:698]: (warning) Either the condition '(obj)==NULL' is redundant or there is possible null pointer dereference: obj. [main/astobj2.c:709] -> [main/astobj2.c:701]: (warning) Either the condition '(obj)==NULL' is redundant or there is possible null pointer dereference: obj. [main/pbx.c:1087] -> [main/pbx.c:1090]: (warning) Either the condition 'node' is redundant or there is possible null pointer dereference: node. [main/sdp_state.c:3192] -> [main/sdp_state.c:3285]: (warning) Either the condition 'udptl' is redundant or there is possible null pointer dereference: udptl. [main/sdp_state.c:3192] -> [main/sdp_state.c:3293]: (warning) Either the condition 'udptl' is redundant or there is possible null pointer dereference: udptl. [main/strings.c:367] -> [main/strings.c:365]: (warning) Either the condition '!buffer' is redundant or there is possible null pointer dereference: buffer. [res/ael/pval.c:3399] -> [res/ael/pval.c:2945]: (warning) Either the condition 'if(exten)' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3399] -> [res/ael/pval.c:2949]: (warning) Either the condition 'if(exten)' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3399] -> [res/ael/pval.c:2950]: (warning) Either the condition 'if(exten)' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3719] -> [res/ael/pval.c:3039]: (warning) Either the condition 'exten' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3719] -> [res/ael/pval.c:3040]: (warning) Either the condition 'exten' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3719] -> [res/ael/pval.c:3687]: (warning) Either the condition 'if(exten&&exten->checked_switch)' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3719] -> [res/ael/pval.c:3688]: (warning) Either the condition 'if(exten&&exten->checked_switch)' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3719] -> [res/ael/pval.c:3706]: (warning) Either the condition 'if(exten&&exten->checked_switch)' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3719] -> [res/ael/pval.c:3707]: (warning) Either the condition 'if(exten&&exten->checked_switch)' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3719] -> [res/ael/pval.c:3726]: (warning) Either the condition 'exten' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3719] -> [res/ael/pval.c:3727]: (warning) Either the condition 'exten' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3793] -> [res/ael/pval.c:3800]: (warning) Either the condition 'exten' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3793] -> [res/ael/pval.c:3801]: (warning) Either the condition 'exten' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3866] -> [res/ael/pval.c:3896]: (warning) Either the condition 'exten' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3866] -> [res/ael/pval.c:3897]: (warning) Either the condition 'exten' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3866] -> [res/ael/pval.c:3901]: (warning) Either the condition 'exten' is redundant or there is possible null pointer dereference: exten. [res/ael/pval.c:3866] -> [res/ael/pval.c:3902]: (warning) Either the condition 'exten' is redundant or there is possible null pointer dereference: exten. [res/res_config_ldap.c:1071] -> [res/res_config_ldap.c:1052]: (warning) Either the condition 'if(initfield&&!strcmp(initfield,var->name))' is redundant or there is possible null pointer dereference: initfield. [res/res_config_pgsql.c:629] -> [res/res_config_pgsql.c:580]: (warning) Either the condition 'if(initfield)' is redundant or there is possible null pointer dereference: initfield. [res/res_endpoint_stats.c:65] -> [res/res_endpoint_stats.c:47]: (warning) Either the condition 'old_snapshot' is redundant or there is possible null pointer dereference: snapshot. [res/res_endpoint_stats.c:65] -> [res/res_endpoint_stats.c:69]: (warning) Either the condition 'old_snapshot' is redundant or there is possible null pointer dereference: old_snapshot. [res/res_format_attr_celt.c:121] -> [res/res_format_attr_celt.c:132]: (warning) Either the condition 'attr1' is redundant or there is possible null pointer dereference: attr1. [res/res_format_attr_celt.c:121] -> [res/res_format_attr_celt.c:134]: (warning) Either the condition 'attr1' is redundant or there is possible null pointer dereference: attr1. [res/res_format_attr_celt.c:121] -> [res/res_format_attr_celt.c:134]: (warning) Either the condition 'attr2' is redundant or there is possible null pointer dereference: attr2. [res/res_format_attr_celt.c:121] -> [res/res_format_attr_celt.c:136]: (warning) Either the condition 'attr2' is redundant or there is possible null pointer dereference: attr2. [res/res_pjsip/pjsip_message_filter.c:113] -> [res/res_pjsip/pjsip_message_filter.c:119]: (warning) Either the condition 'transport_state' is redundant or there is possible null pointer dereference: transport_state. [res/res_pjsip_pubsub.c:640] -> [res/res_pjsip_pubsub.c:662]: (warning) Either the condition 'contact_hdr' is redundant or there is possible null pointer dereference: contact_hdr. [res/res_pjsip_registrar.c:273] -> [res/res_pjsip_registrar.c:272]: (warning) Either the condition '!path_str' is redundant or there is possible null pointer dereference: path_str. [res/res_pktccops.c:633] -> [res/res_pktccops.c:554]: (warning) Either the condition '(cmd==GATE_INFO||cmd==GATE_SET_HAVE_GATEID||cmd==GATE_DEL)&&gate' is redundant or there is possible null pointer dereference: gate. [res/res_rtp_asterisk.c:4071] -> [res/res_rtp_asterisk.c:4063]: (warning) Either the condition '!rtp' is redundant or there is possible null pointer dereference: rtp. [res/res_rtp_asterisk.c:4071] -> [res/res_rtp_asterisk.c:4068]: (warning) Either the condition '!rtp' is redundant or there is possible null pointer dereference: rtp. [tests/test_cdr.c:333] -> [tests/test_cdr.c:339]: (warning) Either the condition 'actual' is redundant or there is possible null pointer dereference: actual. [utils/db1-ast/btree/bt_open.c:407] -> [utils/db1-ast/btree/bt_open.c:401]: (warning) Either the condition 'envtmp?envtmp:"/tmp"' is redundant or there is possible null pointer dereference: envtmp. [utils/db1-ast/btree/bt_seq.c:393] -> [utils/db1-ast/btree/bt_seq.c:394]: (warning) Either the condition '(h=__mpool_get(t->bt_mp,h->prevpg,0))==NULL' is redundant or there is possible null pointer dereference: h. [utils/db1-ast/hash/hash_buf.c:279] -> [utils/db1-ast/hash/hash_buf.c:265]: (warning) Either the condition 'bp?bp->addr:0' is redundant or there is possible null pointer dereference: bp. [utils/db1-ast/hash/hash_buf.c:279] -> [utils/db1-ast/hash/hash_buf.c:268]: (warning) Either the condition 'bp?bp->addr:0' is redundant or there is possible null pointer dereference: bp. [utils/db1-ast/hash/hash_buf.c:279] -> [utils/db1-ast/hash/hash_buf.c:270]: (warning) Either the condition 'bp?bp->addr:0' is redundant or there is possible null pointer dereference: bp. [utils/db1-ast/hash/hash_buf.c:279] -> [utils/db1-ast/hash/hash_buf.c:278]: (warning) Either the condition 'bp?bp->addr:0' is redundant or there is possible null pointer dereference: bp. [utils/extconf.c:3146] -> [utils/extconf.c:2793]: (warning) Either the condition 'withcomments&&cfg' is redundant or there is possible null pointer dereference: cfg. {noformat} By: Ilya Shipitsin (chipitsine) 2018-04-17 02:27:51.053-0500 ok, what's next ? By: Alexander Anikin (may213) 2018-04-17 05:15:22.614-0500 Hi, Specified part of codes is redundant really but not this only. There are redundant codes and hidden bugs, so I will produce complete patch based on cppcheck analysis. By: Ilya Shipitsin (chipitsine) 2018-04-17 05:40:41.869-0500 if you will resolve all cppcheck issues, it would be nice By: Alexander Anikin (may213) 2018-04-17 06:27:56.117-0500 More things to be solved: ooh323c/src/context.c:168]: (warning) Possible null pointer dereference: pctxt [ooh323c/src/memheap.c:626]: (warning) Possible null pointer dereference: pElem [ooh323c/src/memheap.c:689]: (warning) Possible null pointer dereference: pNextElem [ooh323c/src/memheap.c:710]: (warning) Possible null pointer dereference: pNextElem [ooh323c/src/memheap.c:823]: (warning) Possible null pointer dereference: pNextElem [ooh323c/src/memheap.c:1065] -> [ooh323c/src/memheap.c:1067]: (warning) Either the condition 'ppMemHeap!=0' is redundant or there is possible null pointer dereference: ppMemHeap. By: Alexander Anikin (may213) 2018-04-17 06:31:43.285-0500 Ilya, Could you please check on your side with attached patch? Looks like all serious cppcheck warning fixed here. By: Ilya Shipitsin (chipitsine) 2018-04-17 06:40:23.700-0500 sure, I'll have a look in few hours By: Ilya Shipitsin (chipitsine) 2018-04-17 15:23:46.301-0500 well, patch is good By: Alexander Anikin (may213) 2018-04-18 05:26:17.503-0500 Thank you, I will commit and close the issue By: Ilya Shipitsin (chipitsine) 2018-04-18 05:34:49.725-0500 there are several interesting findings left, I'll address them in another patch. commit and close this issue ``` # grep '(error' typescript | grep -v 'Uninitialized variable' [addons/ooh323c/src/ooq931.c:507]: (error) Shifting 64-bit value by 64 bits is undefined behaviour [channels/console_video.c:860]: (error) Invalid number of character '{' when these macros are defined: 'HAVE_FFMPEG;HAVE_VIDEO_CONSOLE'. [ast_expr2.c:2329]: (error) Memory allocation size is negative. [ast_expr2.c:2329]: (error) Invalid malloc() argument nr 1. The value is -1 but the valid values are '0:'. [main/bridge_channel.c:1707]: (error) Null pointer dereference: hook [menuselect/menuselect_curses.c:230]: (error) Undefined behavior: Variable 'buf' is used as parameter and destination in s[n]printf(). [menuselect/menuselect_gtk.c:110]: (error) Buffer is accessed out of bounds. [ael.tab.c:3591]: (error) Invalid number of character '{' when no macros are defined. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: 'AAL_ARGCHECK'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: 'ENABLE_NLS;YYENABLE_NLS'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: 'YYENABLE_NLS'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: 'YYERROR_VERBOSE=0'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: 'YYPRINT'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: '_GNU_SOURCE;_STRING_H;__GLIBC__'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: '_MSC_VER;__C99__FUNC__;__STDC__'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: '_MSC_VER;__C99__FUNC__;__STDC__;lint'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: '_STRING_H;__GLIBC__'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: '__GNUC__'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: '__GNUC__;lint'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: '__STDC__'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: 'lint'. [ael.tab.c:3591]: (error) Invalid number of character '{' when these macros are defined: 'short'. [res/ael/pval.c:3632]: (error) Memory leak: for_loop [res/ael/pval.c:3672]: (error) Memory leak: while_loop [utils/db1-ast/recno/rec_open.c:170]: (error) Invalid number of character '(' when these macros are defined: 'MMAP_NOT_AVAILABLE'. [utils/muted.c:764]: (error) Used file that is not opened. ``` By: Friendly Automation (friendly-automation) 2018-04-24 18:41:06.635-0500 Change 8810 merged by Joshua Colp: chan_ooh323: Fix cppcheck warnings [https://gerrit.asterisk.org/8810|https://gerrit.asterisk.org/8810] By: Friendly Automation (friendly-automation) 2018-04-24 18:56:24.631-0500 Change 8809 merged by Joshua Colp: chan_ooh323: Fix cppcheck warnings [https://gerrit.asterisk.org/8809|https://gerrit.asterisk.org/8809] By: Friendly Automation (friendly-automation) 2018-04-25 04:30:53.856-0500 Change 8811 merged by Joshua Colp: chan_ooh323: Fix cppcheck warnings [https://gerrit.asterisk.org/8811|https://gerrit.asterisk.org/8811]