[Lldb-commits] [PATCH] Enable PipeWindows to support reading with timeout, and fix broken build of llgs.

Zachary Turner zturner at google.com
Thu Dec 4 17:47:32 PST 2014

Instead of IsValid() we could have CanRead() and CanWrite().  In the case
of someone calling Create(), both should return true.  In the case of
someone calling OpenReader() then CanRead() returns true and CanWrite()
returns false.  And simillarly for someone calling OpenWriter().

This way, all methods are always applicable for every type of pipe,
anonymous or named.  Also, when calling any of the methods, the
implementation could close both the existing read fd (if it's valid) and
the existing write fd (if it's valid).  This way there would be no way to
get a Pipe object with handles from two different pipes.

The reason I'm a little leary of multiple implementations is because Read /
Write is implemented identically in all three cases, and that's the primary
operation on a pipe.  So the user doesn't shouldn't have to worry about
what type it is.  Furthermore, it would require creating 8 new source files
(2 headers and 2 cpp files on 2 different platform), for a questionable
benefit.  It seems like making separate classes for each would be similar
to making ReadWriteFile, ReadOnlyFile, and WriteOnlyFile.  Ultimately the
only difference is going to be what flags you pass to the creation method.

On Thu Dec 04 2014 at 5:29:09 PM Oleksiy Vyalov <ovyalov at google.com> wrote:

> >>! In D6538#8, @zturner wrote:
> > I thought about something like that too, but then I wondered if it's
> really
> > necessary.  In Windows we need to open a named pipe internally even when
> > the user requests anonymous pipe, because a named pipe is the only way to
> > achieve something like select.  So that would make that scenario really
> > awkward.
> It looks to me that separate classes for anonymous and named pipe make
> interface more clear.
> There is no dual interpretation for some methods like IsValid(): for
> anonymous pipe it presumes both pipe handles are valid, in case of named
> pipe - it's okay to treat pipe object as valid if at least one (either read
> or write) handle is valid.
> If there is a single pipe class it might be confusing for user which
> methods are applicable for anonymous or named pipes.
> Windows implementation of anonymous pipe may create internally  an
> instance of named pipe and delegate all calls to it.
> > It seems like we can do the same thing as you've suggested but without
> the
> > inheritance.
> >
> > Error Create(llvm::StringRef name);   // name is optional
> > Error OpenReader(llvm::StringRef name);   // name is required
> > Error OpenWriter(llvm::StringRef name);    // name is required
> >
> I propose to make Create/OpenReader/OpenWriter/Delete take a pipe name as
> instance field (initialized in constructor) instead of input parameter.
> Otherwise It may mean that class allows to hold read and write handles from
> different pipes.
> > Error Read(buf, size, bytes_read);
> > Error ReadWithTimeout(buf, size, timeout, bytes_read);
> >
> > I would prefer not having to specify the non-blocking option.  The reason
> > is that if user doesn't specify nonblocking, then ReadWithTimeout is an
> > invalid method to call.  And if they do specify non-blocking, the
> > implementation of Read() changes.
> >
> > But we can always support both methods if we always open the pipe
> > nonblocking.  In read you just implement the block yourself even though
> the
> > pipe is opened with O_NONBLOCK
> Good point.
> http://reviews.llvm.org/D6538
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20141205/7f523ef8/attachment.html>

More information about the lldb-commits mailing list