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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to