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

[Tilghman Lesher]

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