commit f6f84d04b243430687bc3bf1653f99d796e26bae
author Treehugger Robot <treehugger-gerrit@google.com> Thu Jun 09 23:11:02 2022 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Thu Jun 09 23:11:02 2022 +0000
tree 42e88b815dec3ac8a54653d56ed270352b979008
parent b71f7757be924b61d9f9b2d2359f87467b0dfcfb
parent e50efc781aee5a94cf962a91691567532c03c2b9

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

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

Change-Id: Ic3c0ce1161b34d3edc2966d4622fc45460ca335d
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;