[Lldb-commits] [PATCH] D38829: Python: SetOutputFileHandle doesn't work with IOBase

[Pavel Labath via Phabricator via lldb-commits]

labath added subscribers: lldb-commits, labath.
labath reopened this revision.
labath added a comment.
This revision is now accepted and ready to land.

(Please add lldb-commits to future reviews, so that people are aware of what's going on.)

So, the reason why this failed to compile is that android does not have the fopencookie function (neither does windows as far as I am aware). However, looking at the change, it's not clear to me whether we actually need the fopencookie functionality to implement this. Couldn't we just change the File::Read/Write functions to call these directly. Right now they do `if (am_i_backed_by_FILE) fwrite() else write()`. We could add a option for them to be backed by a set of callbacks. An even better option would be to just use virtual functions for this (that's the c++ way of doing cookie callbacks). I think that's what the IOObject hierarchy (which File is a part of) was meant to be used for, although I am not sure it will work out of the box for this case.

And it would be great to see some tests for this.



================
Comment at: lldb/trunk/include/lldb/Host/File.h:65
 
+  File(File &&rhs);
+
----------------
Why are you changing the File to be movable? I don't see the connection between this and the fopencookie part.


================
Comment at: lldb/trunk/source/Host/common/File.cpp:124
+  } else {
+    return (int)wrote;
+  }
----------------
cast seems wrong


================
Comment at: lldb/trunk/source/Host/common/File.cpp:140
+  } else {
+    return (int)read;
+  }
----------------
same here


Repository:
  rL LLVM

https://reviews.llvm.org/D38829