zturner added a comment.
Currently running the test suite, will report back when it's done.
================
Comment at: include/lldb/Host/File.h:442
@@ -440,1 +441,3 @@
+ class SelectInfo
+ {
----------------
labath wrote:
> I am not sure about the location of this class, `File` seems like a bad
> place, especially considering that select does not even work with files
> (sockets only) on Windows. Maybe IOObject.h, or a standalone file?
+1 for making a new file out of this. I would probably just call it `Select.h`
or `SelectInfo.h`. I like `Select` better than `SelectInfo` (as a file name
and as a class name), because `SelectInfo` makes it sound like you're going to
query the class for something, not that it's actually going to perform some
system level operation.
================
Comment at: include/lldb/Host/File.h:493-498
@@ +492,8 @@
+
+ bool read_check;
+ bool write_check;
+ bool error_check;
+ bool read_ready;
+ bool write_ready;
+ bool error_ready;
+ };
----------------
How about a bitfield?
================
Comment at: include/lldb/Host/File.h:500
@@ +499,3 @@
+ };
+ std::map<int, FDInfo> m_fd_map;
+ std::unique_ptr<std::chrono::steady_clock::time_point> m_end_time_ap;
----------------
`llvm::DenseMap` will be more efficient (space-wise, and speed-wise)
================
Comment at: include/lldb/Host/File.h:501
@@ +500,3 @@
+ std::map<int, FDInfo> m_fd_map;
+ std::unique_ptr<std::chrono::steady_clock::time_point> m_end_time_ap;
+ };
----------------
labath wrote:
> I think we use `_up` as a suffix now.
`llvm::Optional<T>` is another option here. It's nice in that it doesn't use a
heap allocation and has better cache performance.
================
Comment at: source/Host/common/File.cpp:19
@@ -11,2 +18,2 @@
#include <errno.h>
----------------
Inside of the `#if defined(_WIN32)` preprocessor section here, you need to add
this line:
`#include <winsock2.h>`
https://reviews.llvm.org/D22950
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
