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

[Miklos Szeredi]

On Fri, 28 May 2010, Changli Gao wrote:
> On Fri, May 28, 2010 at 5:42 PM, Miklos Szeredi <miklos@xxxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/