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

Oleksiy Vyalov ovyalov at google.com
Thu Dec 4 17:29:07 PST 2014


>>! 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






More information about the lldb-commits mailing list