linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Changli Gao <xiaosuo@gmail.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: axboe@kernel.dk, viro@zeniv.linux.org.uk, mszeredi@suse.cz,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 4/4] splice: fix updating sd->pos wrongly
Date: Fri, 28 May 2010 18:55:23 +0800	[thread overview]
Message-ID: <AANLkTikJMLiaUIVohTPy4DLF4XmWsy7LCx0bDheq2aHM@mail.gmail.com> (raw)
In-Reply-To: <E1OHwZC-0002KS-EJ@pomaz-ex.szeredi.hu>

On Fri, May 28, 2010 at 6:13 PM, Miklos Szeredi <miklos@szeredi.hu> wrote:
> 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.

I think you misread my patch. Before checking the sd->pos, sd->pos
already is updated with the value returned by splice_read(), so if in
file is seekabble, sd->pos is non-zero when I checking it.



-- 
Regards,
Changli Gao(xiaosuo@gmail.com)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  reply	other threads:[~2010-05-28 10:55 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-26 14:44 [PATCH v2 4/4] splice: fix updating sd->pos wrongly Changli Gao
2010-05-28  9:42 ` Miklos Szeredi
2010-05-28 10:00   ` Changli Gao
2010-05-28 10:13     ` Miklos Szeredi
2010-05-28 10:55       ` Changli Gao [this message]
2010-05-28 11:11         ` Miklos Szeredi
2010-05-28 11:36           ` Changli Gao
2010-05-28 11:40             ` Miklos Szeredi
2010-05-29 14:09         ` Jamie Lokier
2010-05-29 14:22           ` Changli Gao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AANLkTikJMLiaUIVohTPy4DLF4XmWsy7LCx0bDheq2aHM@mail.gmail.com \
    --to=xiaosuo@gmail.com \
    --cc=axboe@kernel.dk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=mszeredi@suse.cz \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).