[MPlayer-dev-eng] [patch 3/5] Fix stream_write_buffer to make sure all requested bytes are written

[Tobias Diedrich]

Reimar Döffinger wrote:
> On Thu, Jan 06, 2011 at 05:06:33PM +0100, Tobias Diedrich wrote:
> > Index: mplayer-patchset1/stream/stream_file.c
> > ===================================================================
> > --- mplayer-patchset1.orig/stream/stream_file.c	2011-01-06 16:18:13.945756000 +0100
> > +++ mplayer-patchset1/stream/stream_file.c	2011-01-06 16:45:35.911193000 +0100
> > @@ -52,13 +52,32 @@
> >  };
> >  
> >  static int fill_buffer(stream_t *s, char* buffer, int max_len){
> > -  int r = read(s->fd,buffer,max_len);
> > -  return (r <= 0) ? -1 : r;
> > +  int nr = 0;
> > +  int r;
> > +  while (nr < max_len) {
> > +    r = read(s->fd,buffer,max_len);
> > +    if (r < 0)
> > +      return -1;
> > +    max_len -= r;
> > +    buffer += r;
> > +    nr += r;
> > +    if (r == 0)
> > +      break;
> > +  }
> > +  return nr;
> >  }
> 
> Have you seen cases where this is necessary for read? The reason I
> dislike it is because I think it might interfere with the code that
> tries to do a clean exit on CTRL+C.
> Also it looks to me like your code allows a 0 return value (which
> previously was not possible), IIRC that can cause hangs.

Well, given that I was adding retry code for write I thought I
should look at the read codepath too.
Posix spec says that read() too may be interrupted by signals and
then return less than requested (probably won't happen on Linux since
reads should be blocking) and of course it can happen on pipes/fifos.
I'll add a check for 0 before the return.

-- 
Tobias						PGP: http://8ef7ddba.uguu.de