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

[Reimar Döffinger]

On Tue, Jan 04, 2011 at 09:35:04PM +0100, Tobias Diedrich wrote:
> None of the calling sites are checking the return value to see if ALL bytes got written.
> 
> This was causing (very occasionally) problems with mencoder when using output
> pipes AND running under a sandbox or when being straced (ptrace is the culprit)
> Theoretically this problem can happen without pipes or ptrace.
> 
> Original patch by Sang-Uok Kum.
> 
> Signed-off-by: Tobias Diedrich <ranma at google.com>
> 
> Index: mplayer-patchset1/stream/stream.c
> ===================================================================
> --- mplayer-patchset1.orig/stream/stream.c	2010-12-21 20:45:31.157756000 +0100
> +++ mplayer-patchset1/stream/stream.c	2010-12-21 20:48:16.229732000 +0100
> @@ -324,13 +324,16 @@
>  }
>  
>  int stream_write_buffer(stream_t *s, unsigned char *buf, int len) {
> -  int rd;
> -  if(!s->write_buffer)
> +  int rd = 0;
> +  if(!s->write_buffer || len < 0)
>      return -1;
> -  rd = s->write_buffer(s, buf, len);
> -  if(rd < 0)
> -    return -1;
> -  s->pos += rd;
> +  while (rd < len) {
> +    int ret = s->write_buffer(s, buf+rd, len-rd);
> +    if (ret < 0)
> +      return -1;
> +    rd += ret;
> +    s->pos += ret;
> +  }

I think this is a risky place to do it, I'd be more in favour of only
adding an assert here and fix all write_buffer implementations to make
sure this issue never happens.
There is also the issue that if write_buffer ever ends up returning 0
this would become an endless loop.