This revision was automatically updated to reflect the committed changes.
Closed by commit rLLDB355323: Refactor user/group name resolving code (authored
by labath, committed by ).
Herald added a project: LLDB.
Changed prior to commit:
https://reviews.llvm.org/D58167?vs=188448=189169#toc
labath added a comment.
Greg, I believe I've responded to all your comments. If there is anything else
you want me to do here, please let me know.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58167/new/
https://reviews.llvm.org/D58167
jingham added a comment.
This is fine by me, though Greg I think still had an outstanding question. If
he's satisfied, I am.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58167/new/
https://reviews.llvm.org/D58167
___
lldb-commits
labath updated this revision to Diff 188448.
labath marked an inline comment as done.
labath added a comment.
Move the resolver interface into Utility
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58167/new/
https://reviews.llvm.org/D58167
Files:
include/lldb/Host/HostInfoBase.h
labath marked 2 inline comments as done.
labath added inline comments.
Comment at: include/lldb/Host/UserIDResolver.h:9
+
+#ifndef LLDB_HOST_USERIDRESOLVER_H
+#define LLDB_HOST_USERIDRESOLVER_H
zturner wrote:
> I wonder if this class should actually be in Host.
zturner added inline comments.
Comment at: include/lldb/Host/UserIDResolver.h:9
+
+#ifndef LLDB_HOST_USERIDRESOLVER_H
+#define LLDB_HOST_USERIDRESOLVER_H
I wonder if this class should actually be in Host. While some specific
implementation of it might be
labath marked an inline comment as done.
labath added inline comments.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:439
if (uid != UINT32_MAX) {
-std::string name;
-if (HostInfo::LookupUserName(uid, name)) {
+if (auto name
jingham added inline comments.
Comment at:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp:439
if (uid != UINT32_MAX) {
-std::string name;
-if (HostInfo::LookupUserName(uid, name)) {
+if (auto name =
labath added inline comments.
Comment at: include/lldb/Host/UserIDResolver.h:24
+public:
+ typedef uint32_t id_t;
+ virtual ~UserIDResolver(); // anchor
clayborg wrote:
> make this 64 bit for future proofing? And if so, just use lldb::user_id_t?
I think the
labath updated this revision to Diff 186982.
labath marked 13 inline comments as done.
labath added a comment.
Rename things to follow lldb style. All the remaining comments should be either
marked as done, or have a comment explaining why I don't think doing that is a
good idea.
CHANGES SINCE
labath added a comment.
I didn't get around to address the style issues yet, but I wanted to respond to
one high-level comment today:
> 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
>
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:
jingham added a comment.
You are using a mix of llvm & lldb naming conventions for local variables and
arguments and the ivars of UserIDResolver (lots of "Uid", etc...) Probably
better to stick with the lldb convention.
Other than that it looks good to me.
Comment at:
labath created this revision.
labath added reviewers: zturner, clayborg, jingham.
Herald added a subscriber: mgorny.
This creates an abstract base class called "UserIDResolver", which can
be implemented to provide user/group ID resolution capabilities for
various objects. Posix host implement a
14 matches
Mail list logo
