Summary:	ASTERISK-17553: [patch] Dead code - ast_FD_SETSIZE
Reporter:	Olle Johansson (oej)	Labels:	patch
Date Opened:	2011-03-14 08:22:18	Date Closed:
Priority:	Minor	Regression?	No
Status:	Open/New	Components:	General
Versions:		Frequency of Occurrence
Related Issues:
Environment:		Attachments:	( 0) 20110315__issue18969__14.diff.txt ( 1) 20110408__issue18969__162.diff.txt
Description:	There is a lot of code in main/asterisk.c that depends on configure_ran_as_root that aims to set a variable (defined in poll.c) called ast_FD_SETSIZE: In Asterisk 1.6.0 this variable was used in an inline function in select.h - but it is now gone (ast_select()). I can't find the ast_FD_SETSIZE being used anywhere else, so it seems like we have a lot of dead meat hanging around that needs some cleaning up. This could potentially affect configure, main/asterisk.c main/poll.c and select.h (an external reference). ****** ADDITIONAL INFORMATION ****** I could not find a category called "core" or "core/deadcode"... If you can find a better category, please change it.
Comments:	By: Tilghman Lesher (tilghman) 2011-03-14 16:57:04 The problem I ran into with this is that it's impossible to use a define set at runtime to vary the size of a structure (specifically, the bitfield used by select(2).  Yes, some of the code in main/asterisk.c is dead, but the majority of it is not -- it produces important warnings when Asterisk configure was not run as root, to tell the user that Very Bad Things could happen on systems where poll(2) is not available. So yes, you can remove the variable itself, and you can remove the very few lines that set it, but leave the rest of the code alone. By: Olle Johansson (oej) 2011-03-15 11:49:35 Thanks for the fast feedback Tilghman. I'm sorry, but I can not spend any time on this. I just wanted to report it so that it did not disappear from memory (mine in this case) and that someone that felt responsible could clean it up. I have severe locking issues to spend my time on... Ouch. By: Digium Subversion (svnbot) 2011-04-01 05:36:44 Repository: asterisk Revision: 312285 U   branches/1.4/include/asterisk/select.h U   branches/1.4/main/asterisk.c ------------------------------------------------------------------------ r312285 | tilghman | 2011-04-01 05:36:43 -0500 (Fri, 01 Apr 2011) | 7 lines Found some leaking file descriptors while looking at ast_FD_SETSIZE dead code. (issue ASTERISK-17553) Reported by: oej Patches:       20110315__issue18969__14.diff.txt uploaded by tilghman (license 14) ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=312285 By: Olle Johansson (oej) 2011-04-01 05:37:34 Thanks, Tilghman! By: Digium Subversion (svnbot) 2011-04-01 05:51:25 Repository: asterisk Revision: 312287 _U  branches/1.6.2/ U   branches/1.6.2/include/asterisk/select.h U   branches/1.6.2/main/asterisk.c ------------------------------------------------------------------------ r312287 | tilghman | 2011-04-01 05:51:24 -0500 (Fri, 01 Apr 2011) | 14 lines Merged revisions 312285 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.4 ........  r312285 | tilghman | 2011-04-01 05:36:42 -0500 (Fri, 01 Apr 2011) | 7 lines    Found some leaking file descriptors while looking at ast_FD_SETSIZE dead code.    (issue ASTERISK-17553)   Reported by: oej   Patches:         20110315__issue18969__14.diff.txt uploaded by tilghman (license 14) ........ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=312287 By: Digium Subversion (svnbot) 2011-04-01 05:58:46 Repository: asterisk Revision: 312288 _U  branches/1.8/ U   branches/1.8/include/asterisk/select.h U   branches/1.8/main/asterisk.c ------------------------------------------------------------------------ r312288 | tilghman | 2011-04-01 05:58:46 -0500 (Fri, 01 Apr 2011) | 21 lines Merged revisions 312287 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.6.2 ................  r312287 | tilghman | 2011-04-01 05:51:24 -0500 (Fri, 01 Apr 2011) | 14 lines    Merged revisions 312285 via svnmerge from  https://origsvn.digium.com/svn/asterisk/branches/1.4    ........    r312285 | tilghman | 2011-04-01 05:36:42 -0500 (Fri, 01 Apr 2011) | 7 lines        Found some leaking file descriptors while looking at ast_FD_SETSIZE dead code.        (issue ASTERISK-17553)     Reported by: oej     Patches:           20110315__issue18969__14.diff.txt uploaded by tilghman (license 14)  ........ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=312288 By: Digium Subversion (svnbot) 2011-04-01 05:59:33 Repository: asterisk Revision: 312289 _U  trunk/ U   trunk/addons/cdr_mysql.c U   trunk/include/asterisk/select.h U   trunk/main/asterisk.c ------------------------------------------------------------------------ r312289 | tilghman | 2011-04-01 05:59:32 -0500 (Fri, 01 Apr 2011) | 32 lines Merged revisions 312286,312288 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ................  r312286 | tilghman | 2011-04-01 05:44:33 -0500 (Fri, 01 Apr 2011) | 2 lines    Reload must react correctly against a possibly changed table, so dropping the conditional reload flag. ................  r312288 | tilghman | 2011-04-01 05:58:45 -0500 (Fri, 01 Apr 2011) | 21 lines    Merged revisions 312287 via svnmerge from  https://origsvn.digium.com/svn/asterisk/branches/1.6.2    ................    r312287 | tilghman | 2011-04-01 05:51:24 -0500 (Fri, 01 Apr 2011) | 14 lines        Merged revisions 312285 via svnmerge from    https://origsvn.digium.com/svn/asterisk/branches/1.4        ........      r312285 | tilghman | 2011-04-01 05:36:42 -0500 (Fri, 01 Apr 2011) | 7 lines            Found some leaking file descriptors while looking at ast_FD_SETSIZE dead code.            (issue ASTERISK-17553)       Reported by: oej       Patches:             20110315__issue18969__14.diff.txt uploaded by tilghman (license 14)    ........  ................ ................ ------------------------------------------------------------------------ http://svn.digium.com/view/asterisk?view=rev&revision=312289 By: Tilghman Lesher (tilghman) 2011-04-08 01:15:34 This is actually NOT fixed yet.  The commit was in reference to the code, but it did not actually remove the dead code. By: Tilghman Lesher (tilghman) 2011-04-08 02:33:35 New patch makes the code no longer dead, by ensuring that we don't exceed the detected limit. By: Leif Madsen (lmadsen) 2011-04-12 15:02:15 Upgraded to Ready for Testing based on feedback provided by Tilghman.