Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable

2015-03-28 Thread Diederik de Groot

---
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

2015-03-27 Thread Diederik de Groot


 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

2015-03-27 Thread Matt Jordan


 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

2015-03-27 Thread Diederik de Groot


 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

2015-03-27 Thread Diederik de Groot


 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

2015-03-27 Thread Matt Jordan

---
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

Re: [asterisk-dev] [Code Review] 4526: clang compiler warning: -Wunused-value -Wunused-variable -Wunused-const-variable

2015-03-26 Thread Diederik de Groot

---
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

2015-03-26 Thread Matt Jordan


 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

2015-03-26 Thread Matt Jordan

---
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