Re: [PATCH v2 4/4] splice: fix updating sd->pos wrongly

Miklos Szeredi
2010-05-28T10:13:51Z

On Fri, 28 May 2010, Changli Gao wrote:
> On Fri, May 28, 2010 at 5:42 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> > On Wed, 26 May 2010, Changli Gao wrote:
> >> fix updating sd->pos wrongly.
> >>
> >> In error path, we don't need to updating sd->pos, if the file isn't seekable.
> >
> > This patch is nonsense.  Why should we handle sd->pos != 0 case
> > differently?
> >
>
> If the in file isn't seekable, its splice_read won't update *ppos, so
> in the error path, we'd better not change it too. Otherwise, some
> assumption will go wrong.

That may be true, but the patch is still nonsense.

Look, your patch is updating/not updating sd->pos based on whether it
is zero or not. It will prevent updating the position for sockets,
but it will also prevent updating the position for regular files if
the position is zero, which is really not what we want.

Since these patches modify the behavior of splice in subtle ways, it
would be good if you had some test program to verify that you aren't
breaking something.

Thanks,
Miklos