[MPlayer-dev-eng] [PATCH] Use posix_fadvise in stream_file if available

[Reimar Döffinger]

On Sun, Nov 08, 2009 at 02:57:44PM +0100, Tobias Diedrich wrote:
> Hmm on second thought it would overwrite the errno, but this should
> work without needing an additional check:
> 
> static int fill_buffer(stream_t *s, char* buffer, int max_len){
>   int r;
> #ifdef POSIX_FADV_WILLNEED
>   posix_fadvise(s->fd, s->pos + max_len, PREFETCH_LEN,
> POSIX_FADV_WILLNEED);
> #endif
>   r = read(s->fd,buffer,max_len);
>   return (r <= 0) ? -1 : r;
> }

I guess that would be ok.
However the check wouldn't be additional, nor would it overwrite errno:
r = read(s->fd,buffer,max_len);
if (r <= 0)
  return -1;
#ifdef POSIX_FADV_WILLNEED
posix_fadvise(s->fd, s->pos + max_len, PREFETCH_LEN, POSIX_FADV_WILLNEED);
#endif
return r;

> BTW, is there any system where read will return a value different
> from -1 on error?
> If not, the extra check on return is not necessary at all and this
> would work just as well:
> 
> static int fill_buffer(stream_t *s, char* buffer, int max_len){
> #ifdef POSIX_FADV_WILLNEED
>   posix_fadvise(s->fd, s->pos + max_len, PREFETCH_LEN,
> POSIX_FADV_WILLNEED);
> #endif
>   return read(s->fd,buffer,max_len);

You missed that a short read of 0 bytes is translated into an error.
MPlayer's code in general can not handle 0 reads, with your code we'd
almost certainly have an endless loop if that happens.