Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4526/ --- (Updated March 28, 2015, 7:54 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Changes --- Committed in revision 433693 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable
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. 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. - 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable
On March 26, 2015, 12: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. 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. - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4526/#review14855 --- On March 27, 2015, 6:09 a.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, 6:09 a.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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable
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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable
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. Diederik de Groot wrote: 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 Done: Test passes both with and without the patch, both under gcc compiled asterisk as well as clang compiled asterisk: Result PASS. SideNote1: All this work i have been doing was to make asterisk compilable clang, so it should be able to run these tests as well, right. With all the submitted patches in place it compiles and runs in dev-mode as well. SideNote2: Because of the discovered error in this function i guess the test should / would have to be updated for this particular case to prevent future regressions. - 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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com --
Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4526/#review14926 --- Ship it! Ship It! - Matt Jordan On March 27, 2015, 6:09 a.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, 6:09 a.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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4526/ --- 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\nclang compiler warning:-Wunused-value -Wunused-variable -Wunused-const-variable 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 433444 /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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4526/#review14855 --- /branches/13/funcs/func_env.c https://reviewboard.asterisk.org/r/4526/#comment25463 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. - Diederik de Groot On March 26, 2015, 6:32 p.m., Diederik de Groot wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4526/ --- (Updated March 26, 2015, 6:32 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\nclang compiler warning:-Wunused-value -Wunused-variable -Wunused-const-variable 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 433444 /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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable
On March 26, 2015, 12: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. I'd double check that the unit tests still pass with the change. - Matt --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4526/#review14855 --- On March 26, 2015, 12:32 p.m., Diederik de Groot wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4526/ --- (Updated March 26, 2015, 12:32 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\nclang compiler warning:-Wunused-value -Wunused-variable -Wunused-const-variable 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 433444 /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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4526/#review14870 --- /branches/13/apps/app_queue.c https://reviewboard.asterisk.org/r/4526/#comment25498 If these are unused, I'd just remove them. We tend to not #if 0 things out and leave them cluttering the codebase; we usually just delete them. If they are needed, that's what source control history is for :-) This comment would apply to all of the unused static variables in this review. - Matt Jordan On March 26, 2015, 12:32 p.m., Diederik de Groot wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/4526/ --- (Updated March 26, 2015, 12:32 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\nclang compiler warning:-Wunused-value -Wunused-variable -Wunused-const-variable 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 433444 /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 -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev