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().<br><div><br></div><div>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.  </div><div><br></div><div>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.</div><br><div class="gmail_quote">On Thu Dec 04 2014 at 5:29:09 PM Oleksiy Vyalov <<a href="mailto:ovyalov@google.com">ovyalov@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">>>! In D6538#8, @zturner wrote:<br>
> I thought about something like that too, but then I wondered if it's really<br>
> necessary.  In Windows we need to open a named pipe internally even when<br>
> the user requests anonymous pipe, because a named pipe is the only way to<br>
> achieve something like select.  So that would make that scenario really<br>
> awkward.<br>
<br>
It looks to me that separate classes for anonymous and named pipe make interface more clear.<br>
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.<br>
If there is a single pipe class it might be confusing for user which methods are applicable for anonymous or named pipes.<br>
Windows implementation of anonymous pipe may create internally  an instance of named pipe and delegate all calls to it.<br>
<br>
<br>
> It seems like we can do the same thing as you've suggested but without the<br>
> inheritance.<br>
><br>
> Error Create(llvm::StringRef name);   // name is optional<br>
> Error OpenReader(llvm::StringRef name);   // name is required<br>
> Error OpenWriter(llvm::StringRef name);    // name is required<br>
><br>
<br>
I propose to make Create/OpenReader/OpenWriter/<u></u>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.<br>
<br>
> Error Read(buf, size, bytes_read);<br>
> Error ReadWithTimeout(buf, size, timeout, bytes_read);<br>
><br>
> I would prefer not having to specify the non-blocking option.  The reason<br>
> is that if user doesn't specify nonblocking, then ReadWithTimeout is an<br>
> invalid method to call.  And if they do specify non-blocking, the<br>
> implementation of Read() changes.<br>
><br>
> But we can always support both methods if we always open the pipe<br>
> nonblocking.  In read you just implement the block yourself even though the<br>
> pipe is opened with O_NONBLOCK<br>
<br>
Good point.<br>
<br>
<a href="http://reviews.llvm.org/D6538" target="_blank">http://reviews.llvm.org/D6538</a><br>
<br>
<br>
</blockquote></div>