dallison/subspace

Add client name to Client

Closed this issue · 4 comments

It would be useful, from a user stand-point to be able to get the client name from a Client. I have a PR for this, but I can't seem to be able to push the branch (maybe give me sufficient permissions?). Here is the diff:

--- a/client/client.cc
+++ b/client/client.cc
@@ -38,6 +38,7 @@ absl::Status Client::Init(const std::string &server_socket,
 
   Request req;
   req.mutable_init()->set_client_name(client_name);
+  name = client_name;
   Response resp;
   std::vector<toolbelt::FileDescriptor> fds;
   fds.reserve(100);
diff --git a/client/client.h b/client/client.h
index add60fc..3b6fc81 100644
--- a/client/client.h
+++ b/client/client.h
@@ -16,6 +16,7 @@
 #include "toolbelt/fd.h"
 #include "toolbelt/sockets.h"
 #include <functional>
+#include <string>
 #include <sys/poll.h>
 
 namespace subspace {
@@ -278,6 +279,9 @@ public:
   // information unless you know how this works in detail.
   void SetDebug(bool v) { debug_ = v; }
 
+  // Get the client's name, as given to the Init function.
+  const std::string& GetName() const { return name; }
+
 private:
   friend class Server;
   friend class Publisher;
@@ -389,6 +393,7 @@ private:
                              int32_t new_slot_size);
   absl::Status ReloadBuffersIfNecessary(details::ClientChannel *channel);
 
+  std::string name;
   toolbelt::UnixSocket socket_;
   toolbelt::FileDescriptor scb_fd_; // System control block memory fd.
   char buffer_[kMaxMessage];        // Buffer for comms with server over UDS.

Did you create a pull request? I will incorporate this into the code (with a naming fix 'name' -> 'name_').

Also, at the moment when you resize a channel, the old buffers are not unmapped when they are no longer in use. I'm going to add a fix for that. There will be a new release soon.

I've incorporated this into release 1.1.2. I still need to implement buffer munmap when no references after resize. I'll do that this week.

Cool! Thanks! I couldn't create a pull request due to permission denied. I think you have to add me as a contributor on github if you want me to be able to push new branches, aka pull requests.

About the unmap thing, I noticed that bug too, but I was pretty sure you knew about it already. ;)

I've added you as a collaborator. I hadn't noticed the unmap thing until today because I was testing with debug off. It was an easy fix and not really a bug (kind of). Let me know if you see any other bugs. I think it's pretty solid now but there are always more bugs.