commit c1d2c3514c2a214d023530c070e5c2221606d1ae
author Treehugger Robot <treehugger-gerrit@google.com> Thu Jun 09 23:45:21 2022 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Thu Jun 09 23:45:21 2022 +0000
tree e02d53e02e965bba4f3d0039a41f29a53291ee2d
parent b53f8264cebf70bbb38d31ebbf17cf0f49e888ea
parent f6f84d04b243430687bc3bf1653f99d796e26bae

Merge "Fix logical bugs around access() POSIX call" am: e50efc781a am: f6f84d04b2

Original change: https://android-review.googlesource.com/c/platform/bootable/recovery/+/2118747

Change-Id: I76850b86e6e4a207492a6101488a789f6d3087f5
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

recovery-persist.cpp

diff --git a/recovery-persist.cpp b/recovery-persist.cpp
index ad101ed..55699b2 100644
--- a/recovery-persist.cpp
+++ b/recovery-persist.cpp
@@ -77,6 +77,10 @@
   }
 }
 
+static bool file_exists(const char* filename) {
+  return access(filename, R_OK) == 0;
+}
+
 static bool rotated = false;
 
 ssize_t logsave(
@@ -141,7 +145,7 @@
     if (has_cache) {
       // Collects and reports the non-a/b update metrics from last_install; and removes the file
       // to avoid duplicate report.
-      if (access(LAST_INSTALL_FILE_IN_CACHE, F_OK) && unlink(LAST_INSTALL_FILE_IN_CACHE) == -1) {
+      if (file_exists(LAST_INSTALL_FILE_IN_CACHE) && unlink(LAST_INSTALL_FILE_IN_CACHE) == -1) {
         PLOG(ERROR) << "Failed to unlink " << LAST_INSTALL_FILE_IN_CACHE;
       }
 
@@ -152,9 +156,9 @@
       }
     }
 
-    /* Is there something in pmsg? */
-    if (access(LAST_PMSG_FILE, R_OK)) {
-        return 0;
+    /* Is there something in pmsg? If not, no need to proceed. */
+    if (!file_exists(LAST_PMSG_FILE)) {
+      return 0;
     }
 
     // Take last pmsg file contents and send it off to the logsave
@@ -164,18 +168,18 @@
     // For those device without /cache, the last_install file has been copied to
     // /data/misc/recovery from pmsg. Looks for the sideload history only.
     if (!has_cache) {
-      if (access(LAST_INSTALL_FILE, F_OK) && unlink(LAST_INSTALL_FILE) == -1) {
+      if (file_exists(LAST_INSTALL_FILE) && unlink(LAST_INSTALL_FILE) == -1) {
         PLOG(ERROR) << "Failed to unlink " << LAST_INSTALL_FILE;
       }
     }
 
     /* Is there a last console log too? */
     if (rotated) {
-        if (!access(LAST_CONSOLE_FILE, R_OK)) {
-            copy_file(LAST_CONSOLE_FILE, LAST_KMSG_FILE);
-        } else if (!access(ALT_LAST_CONSOLE_FILE, R_OK)) {
-            copy_file(ALT_LAST_CONSOLE_FILE, LAST_KMSG_FILE);
-        }
+      if (file_exists(LAST_CONSOLE_FILE)) {
+        copy_file(LAST_CONSOLE_FILE, LAST_KMSG_FILE);
+      } else if (file_exists(ALT_LAST_CONSOLE_FILE)) {
+        copy_file(ALT_LAST_CONSOLE_FILE, LAST_KMSG_FILE);
+      }
     }
 
     return 0;