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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
