clayborg added inline comments.
================
Comment at: include/lldb/Host/UserIDResolver.h:24
+public:
+ typedef uint32_t id_t;
+ virtual ~UserIDResolver(); // anchor
----------------
make this 64 bit for future proofing? And if so, just use lldb::user_id_t?
================
Comment at: include/lldb/Host/UserIDResolver.h:27
+
+ llvm::Optional<llvm::StringRef> GetUserName(id_t Uid) {
+ return Get(Uid, UidCache, &UserIDResolver::DoGetUserName);
----------------
If we are storing this as an optional std::string, why hand it out as a
StringRef?
================
Comment at: include/lldb/Host/UserIDResolver.h:49-51
+ std::mutex Mutex;
+ Map UidCache;
+ Map GidCache;
----------------
prefix with "m_", make lower case and separate by "_"
================
Comment at: source/Commands/CommandObjectPlatform.cpp:1154
if (platform_sp->GetProcessInfo(pid, proc_info)) {
- ProcessInstanceInfo::DumpTableHeader(ostrm, platform_sp.get(),
- m_options.show_args,
+ ProcessInstanceInfo::DumpTableHeader(ostrm, m_options.show_args,
m_options.verbose);
----------------
Why are we not passing the platform in still? What if we need it for something
else?
================
Comment at: source/Commands/CommandObjectPlatform.cpp:1156
m_options.verbose);
- proc_info.DumpAsTableRow(ostrm, platform_sp.get(),
+ proc_info.DumpAsTableRow(ostrm, platform_sp->GetUserIDResolver(),
m_options.show_args, m_options.verbose);
----------------
Ditto
================
Comment at: source/Host/CMakeLists.txt:23
common/FileCache.cpp
+ common/File.cpp
common/FileSystem.cpp
----------------
remove whitespace change unless it is needed?
================
Comment at: source/Host/CMakeLists.txt:37
common/NativeThreadProtocol.cpp
+ common/NativeWatchpointList.cpp
common/OptionParser.cpp
----------------
remove whitespace change unless it is needed?
================
Comment at: source/Host/CMakeLists.txt:45
common/SocketAddress.cpp
+ common/Socket.cpp
common/StringConvert.cpp
----------------
remove whitespace change unless it is needed?
================
Comment at: source/Target/Process.cpp:281
-void ProcessInstanceInfo::Dump(Stream &s, Platform *platform) const {
- const char *cstr;
+void ProcessInstanceInfo::Dump(Stream &s, UserIDResolver &resolver) const {
if (m_pid != LLDB_INVALID_PROCESS_ID)
----------------
So we are currently only using the platform for the user ID resolving, but it
might be nice to keep the platform as the argument in case we need it for
something else in the future?
================
Comment at: source/Target/Process.cpp:352
-void ProcessInstanceInfo::DumpAsTableRow(Stream &s, Platform *platform,
+void ProcessInstanceInfo::DumpAsTableRow(Stream &s, UserIDResolver &resolver,
bool show_args, bool verbose) const {
----------------
Ditto
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58167/new/
https://reviews.llvm.org/D58167
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
