Debug: Use UniqueFile

Test: m
Test: Device boots
Test: runtest --path frameworks/base/core/tests/coretests/src/com/android/internal/os/DebugTest.java
Change-Id: Ibd31046ceccfd94c1ec7ce2e0c4a748c009cc69f

2 files changed, 71 insertions, 75 deletions

diff --git a/core/jni/Android.mk b/core/jni/Android.mk
index 7e1a0ab06457..bb556107cd6d 100644
--- a/core/jni/Android.mk
+++ b/core/jni/Android.mk
@@ -220,6 +220,7 @@ LOCAL_C_INCLUDES += \
 LOCAL_SHARED_LIBRARIES := \
     libmemtrack \
     libandroidfw \
+    libbase \
     libexpat \
     libnativehelper \
     liblog \
diff --git a/core/jni/android_os_Debug.cpp b/core/jni/android_os_Debug.cpp
index ba9facfd19cf..2a46edf0ef51 100644
--- a/core/jni/android_os_Debug.cpp
+++ b/core/jni/android_os_Debug.cpp
@@ -15,15 +15,7 @@
  */
 
 #define LOG_TAG "android.os.Debug"
-#include "JNIHelp.h"
-#include "jni.h"
-#include <utils/String8.h>
-#include "utils/misc.h"
-#include "cutils/debugger.h"
-#include <memtrack/memtrack.h>
-#include <memunreachable/memunreachable.h>
 
-#include <cutils/log.h>
 #include <fcntl.h>
 #include <inttypes.h>
 #include <stdio.h>
@@ -40,9 +32,26 @@
 #include <iomanip>
 #include <string>
 
+#include "jni.h"
+
+#include "android-base/stringprintf.h"
+#include "cutils/debugger.h"
+#include "cutils/log.h"
+#include "JNIHelp.h"
+#include "memtrack/memtrack.h"
+#include "memunreachable/memunreachable.h"
+#include "utils/misc.h"
+#include "utils/String8.h"
+
 namespace android
 {
 
+using UniqueFile = std::unique_ptr<FILE, decltype(&fclose)>;
+
+static inline UniqueFile MakeUniqueFile(const char* path, const char* mode) {
+    return UniqueFile(fopen(path, mode), fclose);
+}
+
 enum {
     HEAP_UNKNOWN,
     HEAP_DALVIK,
@@ -427,17 +436,13 @@ static void read_mapinfo(FILE *fp, stats_t* stats, bool* foundSwapPss)
 
 static void load_maps(int pid, stats_t* stats, bool* foundSwapPss)
 {
-    char tmp[128];
-    FILE *fp;
-
     *foundSwapPss = false;
 
-    sprintf(tmp, "/proc/%d/smaps", pid);
-    fp = fopen(tmp, "r");
-    if (fp == 0) return;
+    std::string smaps_path = base::StringPrintf("/proc/%d/smaps", pid);
+    UniqueFile fp = MakeUniqueFile(smaps_path.c_str(), "re");
+    if (fp == nullptr) return;
 
-    read_mapinfo(fp, stats, foundSwapPss);
-    fclose(fp);
+    read_mapinfo(fp.get(), stats, foundSwapPss);
 }
 
 static void android_os_Debug_getDirtyPagesPid(JNIEnv *env, jobject clazz,
@@ -519,51 +524,48 @@ static jlong android_os_Debug_getPssPid(JNIEnv *env, jobject clazz, jint pid,
     jlong uss = 0;
     jlong memtrack = 0;
 
-    char tmp[128];
-    FILE *fp;
-
     struct graphics_memory_pss graphics_mem;
     if (read_memtrack_memory(pid, &graphics_mem) == 0) {
         pss = uss = memtrack = graphics_mem.graphics + graphics_mem.gl + graphics_mem.other;
     }
 
-    sprintf(tmp, "/proc/%d/smaps", pid);
-    fp = fopen(tmp, "r");
+    {
+        std::string smaps_path = base::StringPrintf("/proc/%d/smaps", pid);
+        UniqueFile fp = MakeUniqueFile(smaps_path.c_str(), "re");
 
-    if (fp != 0) {
-        while (true) {
-            if (fgets(line, 1024, fp) == NULL) {
-                break;
-            }
+        if (fp != nullptr) {
+            while (true) {
+                if (fgets(line, 1024, fp.get()) == NULL) {
+                    break;
+                }
 
-            if (line[0] == 'P') {
-                if (strncmp(line, "Pss:", 4) == 0) {
-                    char* c = line + 4;
-                    while (*c != 0 && (*c < '0' || *c > '9')) {
-                        c++;
+                if (line[0] == 'P') {
+                    if (strncmp(line, "Pss:", 4) == 0) {
+                        char* c = line + 4;
+                        while (*c != 0 && (*c < '0' || *c > '9')) {
+                            c++;
+                        }
+                        pss += atoi(c);
+                    } else if (strncmp(line, "Private_Clean:", 14) == 0
+                                || strncmp(line, "Private_Dirty:", 14) == 0) {
+                        char* c = line + 14;
+                        while (*c != 0 && (*c < '0' || *c > '9')) {
+                            c++;
+                        }
+                        uss += atoi(c);
                     }
-                    pss += atoi(c);
-                } else if (strncmp(line, "Private_Clean:", 14) == 0
-                        || strncmp(line, "Private_Dirty:", 14) == 0) {
-                    char* c = line + 14;
+                } else if (line[0] == 'S' && strncmp(line, "SwapPss:", 8) == 0) {
+                    char* c = line + 8;
+                    jlong lSwapPss;
                     while (*c != 0 && (*c < '0' || *c > '9')) {
                         c++;
                     }
-                    uss += atoi(c);
+                    lSwapPss = atoi(c);
+                    swapPss += lSwapPss;
+                    pss += lSwapPss; // Also in swap, those pages would be accounted as Pss without SWAP
                 }
-            } else if (line[0] == 'S' && strncmp(line, "SwapPss:", 8) == 0) {
-                char* c = line + 8;
-                jlong lSwapPss;
-                while (*c != 0 && (*c < '0' || *c > '9')) {
-                    c++;
-                }
-                lSwapPss = atoi(c);
-                swapPss += lSwapPss;
-                pss += lSwapPss; // Also in swap, those pages would be accounted as Pss without SWAP
             }
         }
-
-        fclose(fp);
     }
 
     if (outUssSwapPss != NULL) {
@@ -607,12 +609,14 @@ static long get_allocated_vmalloc_memory() {
             NULL
     };
     long size, vmalloc_allocated_size = 0;
-    FILE* fp = fopen("/proc/vmallocinfo", "r");
-    if (fp == NULL) {
+
+    UniqueFile fp = MakeUniqueFile("/proc/vmallocinfo", "re");
+    if (fp == nullptr) {
         return 0;
     }
+
     while (true) {
-        if (fgets(line, 1024, fp) == NULL) {
+        if (fgets(line, 1024, fp.get()) == NULL) {
             break;
         }
         bool valid_line = true;
@@ -628,7 +632,6 @@ static long get_allocated_vmalloc_memory() {
             vmalloc_allocated_size += size;
         }
     }
-    fclose(fp);
     return vmalloc_allocated_size;
 }
 
@@ -652,27 +655,25 @@ enum {
 static long long get_zram_mem_used()
 {
 #define ZRAM_SYSFS "/sys/block/zram0/"
-    FILE *f = fopen(ZRAM_SYSFS "mm_stat", "r");
-    if (f) {
+    UniqueFile mm_stat_file = MakeUniqueFile(ZRAM_SYSFS "mm_stat", "re");
+    if (mm_stat_file) {
         long long mem_used_total = 0;
 
-        int matched = fscanf(f, "%*d %*d %lld %*d %*d %*d %*d", &mem_used_total);
+        int matched = fscanf(mm_stat_file.get(), "%*d %*d %lld %*d %*d %*d %*d", &mem_used_total);
         if (matched != 1)
             ALOGW("failed to parse " ZRAM_SYSFS "mm_stat");
 
-        fclose(f);
         return mem_used_total;
     }
 
-    f = fopen(ZRAM_SYSFS "mem_used_total", "r");
-    if (f) {
+    UniqueFile mem_used_total_file = MakeUniqueFile(ZRAM_SYSFS "mem_used_total", "re");
+    if (mem_used_total_file) {
         long long mem_used_total = 0;
 
-        int matched = fscanf(f, "%lld", &mem_used_total);
+        int matched = fscanf(mem_used_total_file.get(), "%lld", &mem_used_total);
         if (matched != 1)
             ALOGW("failed to parse " ZRAM_SYSFS "mem_used_total");
 
-        fclose(f);
         return mem_used_total;
     }
 
@@ -785,8 +786,8 @@ static void android_os_Debug_getMemInfo(JNIEnv *env, jobject clazz, jlongArray o
 
 static jint read_binder_stat(const char* stat)
 {
-    FILE* fp = fopen(BINDER_STATS, "r");
-    if (fp == NULL) {
+    UniqueFile fp = MakeUniqueFile(BINDER_STATS, "re");
+    if (fp == nullptr) {
         return -1;
     }
 
@@ -797,8 +798,7 @@ static jint read_binder_stat(const char* stat)
 
     // loop until we have the block that represents this process
     do {
-        if (fgets(line, 1024, fp) == 0) {
-            fclose(fp);
+        if (fgets(line, 1024, fp.get()) == 0) {
             return -1;
         }
     } while (strncmp(compare, line, len));
@@ -807,8 +807,7 @@ static jint read_binder_stat(const char* stat)
     len = snprintf(compare, 128, "  %s: ", stat);
 
     do {
-        if (fgets(line, 1024, fp) == 0) {
-            fclose(fp);
+        if (fgets(line, 1024, fp.get()) == 0) {
             return -1;
         }
     } while (strncmp(compare, line, len));
@@ -816,7 +815,6 @@ static jint read_binder_stat(const char* stat)
     // we have the line, now increment the line ptr to the value
     char* ptr = line + len;
     jint result = atoi(ptr);
-    fclose(fp);
     return result;
 }
 
@@ -962,16 +960,15 @@ static void dumpNativeHeap(FILE* fp)
 
     fprintf(fp, "MAPS\n");
     const char* maps = "/proc/self/maps";
-    FILE* in = fopen(maps, "r");
-    if (in == NULL) {
+    UniqueFile in = MakeUniqueFile(maps, "re");
+    if (in == nullptr) {
         fprintf(fp, "Could not open %s\n", maps);
         return;
     }
     char buf[BUFSIZ];
-    while (size_t n = fread(buf, sizeof(char), BUFSIZ, in)) {
+    while (size_t n = fread(buf, sizeof(char), BUFSIZ, in.get())) {
         fwrite(buf, sizeof(char), n, fp);
     }
-    fclose(in);
 
     fprintf(fp, "END\n");
 }
@@ -1001,8 +998,8 @@ static void android_os_Debug_dumpNativeHeap(JNIEnv* env, jobject clazz,
         return;
     }
 
-    FILE* fp = fdopen(fd, "w");
-    if (fp == NULL) {
+    UniqueFile fp(fdopen(fd, "w"), fclose);
+    if (fp == nullptr) {
         ALOGW("fdopen(%d) failed: %s\n", fd, strerror(errno));
         close(fd);
         jniThrowRuntimeException(env, "fdopen() failed");
@@ -1010,10 +1007,8 @@ static void android_os_Debug_dumpNativeHeap(JNIEnv* env, jobject clazz,
     }
 
     ALOGD("Native heap dump starting...\n");
-    dumpNativeHeap(fp);
+    dumpNativeHeap(fp.get());
     ALOGD("Native heap dump complete.\n");
-
-    fclose(fp);
 }