[ruby-core:35584] Re: [Ruby 1.9 - Feature #4532][Open] [PATCH] add IO#pread and IO#pwrite methods

KOSAKI Motohiro <[e-mail]> wrote:
> 2011/3/29 Eric Wong <[e-mail]>:
> > KOSAKI Motohiro <[e-mail]> wrote:
> >> Do we really need to introduce new method? Why can't we overload
> >> IO.read and IO.write?
> >> too complex?
> >
> > IO#read and IO#write take userspace buffers into account which
> > makes no sense with pread/pwrite.
> 
> userspace buffer is implementation detail. no?

It's a very important detail.  Different processes don't share userspace
buffers and Ruby should be able to share buffers with others (non-Ruby
processes), so we must only use the kernel cache.

> And, If pread is always behave as binary mode read method, your
> documentation is much misleading. IMHO.
> 
>  *     f = File.new("testfile")
>  *     f.pread(16, 0)   #=> "This is line one"

Yes, I just copied docs from sysread/syswrite :x  I will update them
in the next patch I post.

> > I'm not a fan of throwing NotImplementedError and faking with lseek() +
> > read()/write() to be even worse since it loses the atomicity guarantee.
> 
> I disagree. I dislike following part of your patch.
> 
> #ifdef HAVE_PREAD
>     rb_define_method(rb_cIO, "pread", rb_io_pread, -1);
> #endif
> 
> This is very wrong style for new method. Eventually,  *all* users need to call
> method_defined? before pread. Just NotImplementedError (or lseek emulation)
> makes much simpler script.

OK, shall I resubmit with adding optional offset argument to
sysread/syswrite and raise NotImplementedError?

Thank you again for your comments!

-- 
Eric Wong