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

[Alexander Strasser]

Hi Reimar!

Reimar Döffinger wrote:
> On Sat, Nov 17, 2012 at 01:01:58AM +0100, Alexander Strasser wrote:
> > @@ -144,8 +149,20 @@ static int readline(char *buf,int max,struct stream_priv_s *ctl)
> >  	}
> >        }
> >        if (max == 1) {
> > -	*buf = '\0';
> > -	break;
> > +        int found_eol = 0;
> > +
> > +        while (ctl->cavail > 0) { // skip remaining response line
> > +          found_eol = *ctl->cget == '\n';
> > +          --ctl->cavail; ++ctl->cget;
> > +
> > +          if (found_eol) {
> > +            break;
> > +          }
> > +        }
> > +
> > +        if (found_eol) {
> > +	  break;
> > +        }
> 
> I find this code quite non-obvious.

  I am glad you had a look at this. I somewhat disliked the double break...

> Maybe instead
> char *eol = memchr(ctl->cget, ctl->cavail);
> if (eol) {
>   eol++;
>   ctl->cavail -= eol - ctl->cget;
>   ctl->cget = eol;
>   break;
> }
> // read more data to find end of current line

  I replaced the code now with this:
--8<--
  char *q = memchr(ctl->cget, '\n', ctl->cavail);

  if (q) { // found EOL: update state and return
    ++q;
    ctl->cavail -= q - ctl->cget;
    ctl->cget = q;

    break;
  }

  // receive more data to find end of current line
  ctl->cget = ctl->cput;
-->8--

> I also wonder if we should do something against the
> possibility that a hacked server could keep us in the
> loop forever...

  Having a limit on how many bytes to look with one call of
readline() might be a good idea. I will try to come up with
something!

[...]

  Alexander