Fix memtrack_helper for newer versions Android.

tools/android/memtrack_helper/memtrack_helper.c

Index: tools/android/memtrack_helper/memtrack_helper.c
diff --git a/tools/android/memtrack_helper/memtrack_helper.c b/tools/android/memtrack_helper/memtrack_helper.c
index 9ce05fdea509945fea5d84b86b8fbe94e20a64bc..cf33197fff0e18528441867e15d57f10cf79684a 100644
--- a/tools/android/memtrack_helper/memtrack_helper.c
+++ b/tools/android/memtrack_helper/memtrack_helper.c
@@ -8,17 +8,13 @@
 
 #include <dlfcn.h>
 #include <errno.h>
-#include <fcntl.h>
-#include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <unistd.h>
 #include <sys/socket.h>
 #include <sys/un.h>
 #include <sys/stat.h>
 #include <sys/types.h>
-#include <sys/wait.h>
 
 /*
  * This is a helper daemon for Android which makes memtrack graphics information
@@ -29,7 +25,8 @@
 static const char kShutdownRequest = 'Q';
 
 // See $ANDROID/system/core/include/memtrack/memtrack.h.
-typedef void* memtrack_proc_handle;
+struct memtrack_proc;

[Primiano Tucci (use gerrit), 2016/12/14 15:51:16]

why this? we don't care what's the real type of the handle.

[kraynov, 2016/12/15 15:43:47]

Done.

+typedef struct memtrack_proc* memtrack_proc_handle;
 typedef int (*memtrack_init_t)(void);
 typedef memtrack_proc_handle (*memtrack_proc_new_t)(void);
 typedef void (*memtrack_proc_destroy_t)(memtrack_proc_handle);
@@ -66,7 +63,8 @@ static void send_shutdown_request(struct sockaddr_un* addr) {
 static void handle_one_request(int client_sock) {
   char buf[32];
   char response[4096] = "";
-  ssize_t rsize = recv(client_sock, buf, sizeof(buf), 0);
+  ssize_t rsize = recv(client_sock, buf, sizeof(buf) - 1, 0);
+  buf[rsize] = '\0';

[Primiano Tucci (use gerrit), 2016/12/14 15:51:16]

now this will crash if recv fails and rsize = -1
I think what you want here is:

ssize_t rsize = recv(client_sock, buf, sizeof(buf) - 1, 0)
buf[sizeof(buf) - 1] = '\0';

So you are always guaranteed to have at least a NUL at the end

[kraynov, 2016/12/15 15:43:48]

Done.

 
   if (rsize < 1)
     return;
@@ -75,7 +73,7 @@ static void handle_one_request(int client_sock) {
     exit(EXIT_SUCCESS);
 
   pid_t pid = -1;
-  if (sscanf(buf, "%d", &pid) != 1)
+  if (sscanf(buf, "%d", &pid) != 1 || pid < 0)
     return send_response(client_sock, "ERR invalid pid");
 
   memtrack_proc_handle handle = memtrack_proc_new();
@@ -97,12 +95,12 @@ static void handle_one_request(int client_sock) {
                             memtrack_proc_graphics_pss(handle));
   }
   if (memtrack_proc_gl_total) {
-    response_ptr +=
-        sprintf(response_ptr, "gl_total %zd\n", memtrack_proc_gl_total(handle));
+    response_ptr += sprintf(response_ptr, "gl_total %zd\n",
+                            memtrack_proc_gl_total(handle));
   }
   if (memtrack_proc_gl_pss) {
-    response_ptr +=
-        sprintf(response_ptr, "gl_pss %zd\n", memtrack_proc_gl_pss(handle));
+    response_ptr += sprintf(response_ptr, "gl_pss %zd\n",
+                            memtrack_proc_gl_pss(handle));
   }
   if (memtrack_proc_other_total) {
     response_ptr += sprintf(response_ptr, "other_total %zd\n",
@@ -117,38 +115,6 @@ static void handle_one_request(int client_sock) {
   send_response(client_sock, response);
 }
 
-static void daemonize() {
-  pid_t pid;
-
-  pid = fork();
-  if (pid < 0)
-    exit_with_failure("fork");
-  if (pid > 0) {
-    /* Terminate the main process attached to TTY once the daemon re-forks. */
-    int ignore;
-    wait(&ignore);
-    exit(EXIT_SUCCESS);
-  }
-
-  if (setsid() == -1)
-    exit_with_failure("setsid");
-
-  chdir("/");
-  umask(0);
-  close(STDIN_FILENO);
-  close(STDOUT_FILENO);
-  close(STDERR_FILENO);
-  open("/dev/null", O_RDONLY);
-  open("/dev/null", O_WRONLY);
-  open("/dev/null", O_RDWR);
-
-  pid = fork();
-  if (pid < 0)
-    exit_with_failure("fork");
-  if (pid > 0)
-    exit(EXIT_SUCCESS);
-}
-
 int main(int argc, char** argv) {
   int res;
 
@@ -180,8 +146,7 @@ int main(int argc, char** argv) {
   memtrack_proc_other_pss =
       (memtrack_proc_other_pss_t)dlsym(libhandle, "memtrack_proc_other_pss");
 
-  if (!memtrack_init || !memtrack_proc_new || !memtrack_proc_destroy ||
-      !memtrack_proc_get) {
+  if (!memtrack_proc_new || !memtrack_proc_destroy || !memtrack_proc_get) {
     exit_with_failure("dlsym() libmemtrack.so");
   }
 
@@ -208,15 +173,30 @@ int main(int argc, char** argv) {
   if (res)
     exit_with_failure("bind");
 
-  if (argc > 1 && strcmp(argv[1], "-d") == 0)
-    daemonize();
-
-  res = memtrack_init();
-  if (res == -ENOENT) {
-    exit_with_failure("Unable to load memtrack module in libhardware. "
-                      "Probably implementation is missing in this ROM.");
-  } else if (res != 0) {
-    exit_with_failure("memtrack_init() returned non-zero status.");
+  if (argc > 1 && strcmp(argv[1], "-d") == 0) {
+    int res = daemon(0, 0);
+    if (res != 0)
+      exit_with_failure("Failed to daemonize.");
+  }
+  log_pid();
+
+  if (memtrack_init) {
+    res = memtrack_init();
+    if (res == -ENOENT) {
+      exit_with_failure("Unable to load memtrack module in libhardware. "
+                        "Probably implementation is missing in this ROM.");
+    } else if (res != 0) {
+      exit_with_failure("memtrack_init() returned non-zero status.");
+    }
+  } else {
+    // memtrack_init() has been removed in newer versions of libmemtrack and
+    // replaced with lazy initialization. This code triggers that.
+    memtrack_proc_handle handle = memtrack_proc_new();
+    if (!handle)
+      exit_with_failure("memtrack_proc_new() failed.");
+    if (memtrack_proc_get(handle, getpid()) != 0)

[Primiano Tucci (use gerrit), 2016/12/14 15:51:16]

Hmm I'd remove this. Since this is a non-(Java)-managed  process I can see this
failing in the future.
Just proc_new should be enough as a smoke test.

[kraynov, 2016/12/15 15:43:47]

Acknowledged.

+      exit_with_failure("memtrack_proc_get() returned non-zero status.");
+    memtrack_proc_destroy(handle);
   }
 
   if (listen(server_fd, 128 /* max number of queued requests */))