[Lldb-commits] [PATCH] Enhance the Pipe interface for better portability

Oleksiy Vyalov ovyalov at google.com
Tue Dec 16 17:51:59 PST 2014


================
Comment at: source/Host/posix/ConnectionFileDescriptorPosix.cpp:297
@@ -295,1 +296,3 @@
+    Error result = m_pipe.Write("i", 1, bytes_written);
+    return result.Success();
 }
----------------
zturner wrote:
> ovyalov wrote:
> > zturner wrote:
> > > ovyalov wrote:
> > > > return (result.Success() && bytes_written == 1) ?
> > > It seems redundant to check for bytes_written == 1.  A postcondition of Pipe::Write should be that result.Success() implies bytes_written == bytes_requested.
> > Quote from PipePosix::Write:
> > 
> > 
> > ```
> > int result = write(fd, buf, num_bytes);
> >         if (result >= 0)
> >             bytes_written = result;
> > ```
> > 
> > So, if write returns 0 Write still will be successful.
> Is it possible to re-write it in such a way that Read() blocks until either success, failure, or EOF, and Write() blocks until either success or failure?
> 
> I'm not familiar with all the posix intricacies, but a quick look at the man page suggests that the only case this might happen is when the pipe's output buffer is full.  But shouldn't we require that Pipe::Write() blocks until the data is written or an error occurs?  Of course, the function might need to be modified in order to guarantee that (similarly for read).  
> 
> If we can agree that the functions should behave that way (even if they currently aren't), then maybe you could do that at the same time you implement the fifo and ReadWithTimeout logic?   It might be easier for someone who is more familiar with linux than I am.
Yes, I think it makes to implement Read/Write in the way that you suggested and such change shouldn't affect the code that potentially anticipates possible partial read/writes.
It's definitely should be done in another CL - I can take on this later.

http://reviews.llvm.org/D6686

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the lldb-commits mailing list