commit b9419caeb6d17d3532280e9f874630192cd22e25
author Treehugger Robot <treehugger-gerrit@google.com> Fri Jun 10 00:33:50 2022 +0000
committer Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> Fri Jun 10 00:33:50 2022 +0000
tree e02d53e02e965bba4f3d0039a41f29a53291ee2d
parent f3a51bf0bb579e7e410f33d7053b69bc8b41e728
parent d72b556518565e536f0d84ab2c7c69ea84809322

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

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

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