commit 0cd72a9b62fcefc729e7f0a3c1bd3a7dfe9e5758
author Artem Yudin <arty@google.com> Tue Jun 07 16:32:44 2022 -0700
committer Artem Yudin <arty@google.com> Tue Jun 07 17:19:23 2022 -0700
tree ebff2b4ea822450212f6784294457b9ae1f05c4f
parent 94bb8a80c7cb09adacbba8178808de1d102d6dd6

Fix logical bugs around access() POSIX call

According to docs of access():
If the specified access is permitted, access() returns 0.
If the given file cannot be accessed in the specified way, access() returns -1

See my comment: https://android-review.googlesource.com/c/platform/bootable/recovery/+/883392/comments/e9b76d97_f4cab88d

Test: builds
Bug: 234887937
Change-Id: I5712d61e967566246603f23a62e2c7b708130b0d

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;