[asterisk-dev] [Code Review] Make ast_carefulwrite() ... be more careful

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/99/#review251
-----------------------------------------------------------


So here's a basic architectural question for you. Why is it that we write first and then poll the file descriptor only if we need to write more data? Wouldn't it make more sense and in fact be more "careful" to poll on the file descriptor and then write when the descriptor is ready?


/branches/1.4/main/utils.c
<http://reviewboard.digium.com/r/99/#comment503>

    This is strictly my opinion, but instead of having do {} while (1), why don't you instead do something like:
    
    while((res = poll(&pfd, 1, timeoutms)) <= 0))) {
        /* Error checking */
    }


- Mark


On 2008-12-18 14:52:14, Russell Bryant wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.digium.com/r/99/
> -----------------------------------------------------------
> 
> (Updated 2008-12-18 14:52:14)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> This patch was written to address some cases where users of the Asterisk Manager Interface could observe partial writes of messages.  In Asterisk 1.4, the function ast_carefulwrite() is used to write data to the socket.  While the function was sort of careful, it was not careful enough.  This patch addresses the following issues:
> 
> 1) It did not previously handle an error of EINTR from write().
> 2) It did not handle EINTR or EGAIN errors from poll().
> 
> 
> This addresses bug 13546.
>     http://bugs.digium.com/view.php?id=13546
> 
> 
> Diffs
> -----
> 
>   /branches/1.4/main/utils.c 165762 
> 
> Diff: http://reviewboard.digium.com/r/99/diff
> 
> 
> Testing
> -------
> 
> I connected to the AMI, made sure I could still log in, and observed normal looking responses and events.
> 
> 
> Thanks,
> 
> Russell
> 
>