Hi Godbach,
On Wed, Oct 09, 2013 at 11:28:32PM +0800, Godbach wrote:
> Hi Willy,
>
> It seems that the loop will be only excuted once.
>
> The related codes as below(src/stream_interface.c):
> > static int si_conn_send_loop(struct connection *conn)
> > {
> > ...
> > while (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_WR_SH | ...))) {
> > ...
> > ret = conn->xprt->snd_buf(conn, chn->buf, send_flag);
> > if (ret <= 0)
> > break;
> >
> > chn->flags |= CF_WRITE_PARTIAL;
> >
> > if (!chn->buf->o) {
> > ...
> > }
> >
> > /* if some data remain in the buffer, it's only because the
> > * system bufers are full, so we don't want to loop again.
> > */
> > break;
> > } /* while */
> >
> > if (conn->flags & CO_FL_ERROR)
> > return -1;
> >
> > return 0;
> > }
> Since there is a 'break' with no condition in the last line of while
> loop, the loop will
> be only excuted once. It just looks like a 'if' as below:
> > - while (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_WR_SH | ...))) {
> > + if (!(conn->flags & (CO_FL_ERROR | CO_FL_SOCK_WR_SH | ...))) {
Yes I remember about this change, it happened one year ago in the
following commit :
ed7f836 BUG/MINOR: stream_interface: don't loop over ->snd_buf()
In 1.4 and 1.5 before the connection rework, the loop was used to
call send() over each buffer's half (when data was wrapped around
at the end). Now the transport protocol's send() function does the
job and we don't need to loop anymore.
However, I left the code as is because not having to replace the
breaks by gotos etc... reduced the amount of changes at the time
(we were at a stage where the new connections were not completely
stabilized yet).
Now it's OK. Feel free to send a cleanup patch if you want (and
change the comment at the top of the loop).
Best regards,
Willy
