[MPlayer-dev-eng] [PATCHv2 3/8] stream ftp: readline: Always try to read complete lines

[Alexander Strasser]

Reimar Döffinger wrote:
> On 18 Nov 2012, at 22:38, Alexander Strasser <eclipse7 at gmx.net> wrote:
> > Reimar Döffinger wrote:
> >> 
> >> I forgot to ask: Did you check it is okay to skip
> >> the cget == cput check and resetting all fields code
> >> when we break the loop here?
> > 
> >  I did. I think it is not any different to breaking out
> > if we found EOL in the range of the provided buffer in the
> > if-block above. On the next call we will get into the
> > if (cget == cput)-block if we exhausted the recv buffer on
> > the call before. We may get into the changed if (max == 1)-block
> > too but it will turn out be a NOP.
> > 
> >  Talking about that should I change the condition of the
> > first if (ctl->cavail > 0)-block to if (ctl->cavail > 0 && max > 1) ?
> > And that of the 2nd to if (max == 1) to if (ctl->cavail > 0 && max == 1) ?
> > IOW do not try to fill the provided line buffer if it is full already
> > and do not try to search for EOL if the recv buffer is exhausted.
> > AFAICT it works as well without those extra conditions, so I am not
> > really sure about adding them.
> 
> I think the code should be fine as is, though it might be possible to make it more obvious. But one thing at a time.
> 
> >  Also find attached a text file visualizing some cases with
> > the changed behavior. Maybe you can see a flaw in my thinking.
> 
> I don't think there is one, I just didn't spend enough time to be sure myself, so I asked if you did.

  I must agree that the code is not as obvious as I'd like it to be.
Especially this makes it so easy to underestimate the way it works...

  So by any means I am glad for everyone challenging my changes!

  Alexander