[Lldb-commits] [PATCH] D25783: [Host] handle short reads and writes

[Pavel Labath via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D25783#574873, @zturner wrote:

> In https://reviews.llvm.org/D25783#574860, @labath wrote:
>
> > > This can happen with any number of bytes and at any time.  `write`, `read`, and all other related functions will return a non-negative value indicating the number of bytes successfully read/written, which will be less than the number requested.
> >
> > Except if fd refers to a pipe, in which case writes of up to PIPE_MAX will be atomic.
> >
> > Also, I just noticed another problem. What if the fd does not refer to an actual file, but a non-seekable file system object (named pipe, domain socket, ...). Will the lseek work on that? I have no idea. But, I think you're implementing a broken API to save a couple of lines of code.
>
>
> In that case the user should be using `Host/Pipe` or a more suitable class, this class is already unsuitable just on the grounds that it exposes a method (the offset version) that requires a seekable device.  I'd even be fine asserting in the constructor if the device is not seekable.


If I have a string, the only way to know whether it refers to a seekable file is to open it and do an fstat() on the descriptor. And then I'm in the business of trying to decipher `stat.st_mode` to figure out whether to instantiate a Pipe, Socket, or a File object (do we want a CharacterDevice class ?). Check the 'a' attribute of https://linux.die.net/man/1/chattr for a fun corner case. Files in the proc filesystem are also very amusing. They are perfectly well readable and writable and appear as regular files in most aspects, but if you do an fstat() on them, you'll see their size as zero. I have no idea what would happen if I did a pwrite on a file in /proc -- it would probably work just fine as linux seems to ignore the offsets when they don't make sense, but that's not something I want to rely on.

> IDK, I'm the Windows guy so this isn't really my call, but I think part of the reason why the test suite is so flaky and bugs are so hard to track down sometimes is because we don't make assumptions.  If we find some code that is clearly broken without a given set of assumptions, I think making it break "even more" without those assumptions is not only fine, but even desirable so that problems become easier to track down.

I'm all for assertions, but I don't think this is a good example. In UNIX world (and linux is an extreme example of this) a great many things can be represented by a file-like objects (hell, you can even receive signals over a file descriptor, although I don't think you can assign a *name* to that), and here we should make as few assumptions as possible. The safest route for me seems:

- File::Write -> ::write
- File::Write(offset) -> ::pwrite

if pwrite fails on some crazy file, then so be it (you have to expect it to fail for other reasons anyway). It will also be faster, as Write() will do only one syscall instead of three. With a bit of clever programming, I think we can make the code reasonably small as well.


https://reviews.llvm.org/D25783