[asterisk-dev] [Code Review] fix for double close of file descriptors raised in issue 16305

Tilghman Lesher tlesher at digium.com
Mon Nov 30 12:54:24 CST 2009


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/436/#review1278
-----------------------------------------------------------



/trunk/apps/app_externalivr.c
<https://reviewboard.asterisk.org/r/436/#comment2942>

    I'd be fine with using NULL here, as long as you check that condition before dereferencing it.



/trunk/apps/app_externalivr.c
<https://reviewboard.asterisk.org/r/436/#comment2943>

    You'd be better off checking if the file descriptor was >=0, since negative numbers are guaranteed to be invalid, but 0 is a perfectly valid FD.



/trunk/apps/app_externalivr.c
<https://reviewboard.asterisk.org/r/436/#comment2944>

    Use -1.  0 is a valid FD.



/trunk/apps/app_externalivr.c
<https://reviewboard.asterisk.org/r/436/#comment2945>

    Ditto.



/trunk/apps/app_externalivr.c
<https://reviewboard.asterisk.org/r/436/#comment2946>

    Ditto.


- Tilghman


On 2009-11-30 11:58:47, thedavidfactor wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/436/
> -----------------------------------------------------------
> 
> (Updated 2009-11-30 11:58:47)
> 
> 
> Review request for Asterisk Developers and Chris Tooley.
> 
> 
> Summary
> -------
> 
> a reported raised an issue where EIVR was closing FDs used by AGI when under heavy load. This modification was designed to prevent EIVR from closing an FD if the FD had already been closed.
> 
> 
> This addresses bug 16305.
>     https://issues.asterisk.org/view.php?id=16305
> 
> 
> Diffs
> -----
> 
>   /trunk/apps/app_externalivr.c 230584 
> 
> Diff: https://reviewboard.asterisk.org/r/436/diff
> 
> 
> Testing
> -------
> 
> reporter attached patch that had been tested to fix issue, I have tested this code and it doesn't seem to break any existing functionality
> 
> 
> Thanks,
> 
> thedavidfactor
> 
>




More information about the asterisk-dev mailing list