[Home]

Summary:ASTERISK-16105: ast_close_fds_above_n off-by-one's
Reporter:Walter Doekes (wdoekes)Labels:
Date Opened:2010-05-17 01:18:06Date Closed:2010-05-17 13:44:55
Priority:TrivialRegression?No
Status:Closed/CompleteComponents:Core/General
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:
Description:Hi,

I was checking out the changes in 1.6.2.8rc1 and saw the ast_close_fds_above_n edit in r256483.

http://svn.asterisk.org/svn/asterisk/branches/1.6.2/main/app.c

The function is called "above_n". Shouldn't then:
if ((x = strtol(entry->d_name, &result, 10)) && x >= n) {
be:
if ((x = strtol(entry->d_name, &result, 10)) && x > n) {


And, related, the usage in res_agi seems wrong too:

# wgrep . ast_close_fds_above_n | sed -e '/:void/d;s/[[:blank:]]\+//'
./apps/app_voicemail.c:ast_close_fds_above_n(STDOUT_FILENO);
./apps/app_dahdiras.c:ast_close_fds_above_n(STDERR_FILENO);
./apps/app_festival.c:ast_close_fds_above_n(0);
./apps/app_externalivr.c:ast_close_fds_above_n(STDERR_FILENO);
./apps/app_ices.c:ast_close_fds_above_n(STDERR_FILENO);
./apps/app_nbscat.c:ast_close_fds_above_n(STDERR_FILENO);
./apps/app_mp3.c:ast_close_fds_above_n(STDERR_FILENO);
./main/asterisk.c:ast_close_fds_above_n(STDERR_FILENO);
./main/asterisk.c:ast_close_fds_above_n(0);
./res/res_musiconhold.c:ast_close_fds_above_n(STDERR_FILENO);
./res/res_agi.c:ast_close_fds_above_n(STDERR_FILENO + 1);

# grep -B1 ast_close_fds_above_n ./res/res_agi.c
/* Close everything but stdin/out/error */
ast_close_fds_above_n(STDERR_FILENO + 1);
That should probably be ast_close_fds_above_n(STDERR_FILENO) like the others.


Regards,
Walter Doekes
OSSO B.V.
Comments:By: Leif Madsen (lmadsen) 2010-05-17 13:35:04

@tilghman: fyi, the commit revision referenced in this issue was committed by you.
By: Tilghman Lesher (tilghman) 2010-05-17 13:44:02

I'm fixing the first, but if you read the context in the second, you'll see that it is NOT wrong.
By: Digium Subversion (svnbot) 2010-05-17 13:44:54

Repository: asterisk
Revision: 263587

U   branches/1.6.2/main/app.c

------------------------------------------------------------------------
r263587 | tilghman | 2010-05-17 13:44:53 -0500 (Mon, 17 May 2010) | 4 lines

Don't close 'n', just close 'above_n'.
(closes issue ASTERISK-16105)
Reported by: wdoekes

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=263587