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

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Wed Oct 19 13:52:23 PDT 2016


clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Just a few questions on why we are calling lseek in the read and write functions. See inlined comments.



================
Comment at: source/Host/common/File.cpp:405
 
-    if (bytes_read == 0) {
-      if (::feof(m_stream))
-        error.SetErrorString("feof");
-      else if (::ferror(m_stream))
-        error.SetErrorString("ferror");
-      num_bytes = 0;
-    } else
-      num_bytes = bytes_read;
-  } else {
-    num_bytes = 0;
-    error.SetErrorString("invalid file handle");
-  }
-  return error;
+  off_t offset = lseek(fd, 0, SEEK_CUR);
+  return Read(buf, num_bytes, offset);
----------------
Why are we calling lseek when we are passing the offset into the read below?

Shouldn't this just be:
```
off_t offset = 0;
```


================
Comment at: source/Host/common/File.cpp:416
 
-  ssize_t bytes_written = -1;
-  if (DescriptorIsValid()) {
-    do {
-      bytes_written = ::write(m_descriptor, buf, num_bytes);
-    } while (bytes_written < 0 && errno == EINTR);
-
-    if (bytes_written == -1) {
-      error.SetErrorToErrno();
-      num_bytes = 0;
-    } else
-      num_bytes = bytes_written;
-  } else if (StreamIsValid()) {
-    bytes_written = ::fwrite(buf, 1, num_bytes, m_stream);
-
-    if (bytes_written == 0) {
-      if (::feof(m_stream))
-        error.SetErrorString("feof");
-      else if (::ferror(m_stream))
-        error.SetErrorString("ferror");
-      num_bytes = 0;
-    } else
-      num_bytes = bytes_written;
-
-  } else {
-    num_bytes = 0;
-    error.SetErrorString("invalid file handle");
+  off_t offset = lseek(fd, 0, SEEK_CUR);
+  if (m_options & File::eOpenOptionAppend) {
----------------
Why are we calling lseek here? We specify the offset to the Write below and that function should do the right thing with the offset. 

Shouldn't this just be:
```
off_t offset = 0;
```


https://reviews.llvm.org/D25783





More information about the lldb-commits mailing list