[Lldb-commits] [PATCH] D28305: [Host] Handle short reads and writes, take 3

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: zturner, clayborg.
labath added a subscriber: lldb-commits.
Herald added a subscriber: mgorny.

This is a rework of https://reviews.llvm.org/D25783, which was blocked by my objections to using pwrite
wholesale. This version keeps the benefits of the code simplification in the
previous version, but keeps the distinction between write/pwrite system calls
and their windows equivalents. Besides allowing appends to be atomic, I believe
this actually makes the code simpler(*), as there is no need to do manual
lseeks() to get the file positions.

I had made the retry logic less aggressive than previously, as it was hanging
the test suite - the problem was parts of the codebase were depending on read
returning with a non-full buffer, for example when reading from the inferior
process pseudo-terminal (/dev/pts/ptmx). So, now, I only retry the read if the
user requested a read larger than READ_MAX, and the syscall returned READ_MAX
bytes, which I believe is the original problem we were trying to solve.

This does not make the API particularly nice, but I am not actually
changing that - it was always that. I believe a good cleanup here would be to
let the user specify whether he wants to read the full buffer, or whether he
will be satisfied with a partial read -- a behaviour we already needed in
AdbClient, which is currently rolling out its own implementation of that.

I also fixed some typos in the previous version, and added unit tests for the
modified functions.

(*) It turns out there are windows equivalents for the posix semantics of file

  append - just pass FFFF... as file position. This is even automatic if the
  file was opened with the appropriate CreateFile flags - a thing I could not do
  immediately, as we are using a different function (_wsopen_s) to open files.


https://reviews.llvm.org/D28305

Files:
  include/lldb/Host/File.h
  source/Host/common/File.cpp
  unittests/Host/CMakeLists.txt
  unittests/Host/FileTest.cpp
  unittests/Host/Inputs/Read.test

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D28305.83084.patch
Type: text/x-patch
Size: 21498 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20170104/9d3ee712/attachment-0001.bin>