Merge changes from topic 'fstab_relocation'

* changes:
  init: replacing fs_mgr_read_fstab() with fs_mgr_read_fstab_default()
  fs_mgr: support reading fstab file from /odm or /vendor partition
  fs_mgr: add fs_mgr_read_fstab_with_dt() API

fs_mgr/fs_mgr_fstab.cpp

@@ -404,7 +404,7 @@ bool is_dt_compatible() {
     return false;
 }
 
-struct fstab *fs_mgr_read_fstab_file(FILE *fstab_file)
+static struct fstab *fs_mgr_read_fstab_file(FILE *fstab_file)
 {
     int cnt, entries;
     ssize_t len;
@@ -540,20 +540,59 @@ struct fstab *fs_mgr_read_fstab_file(FILE *fstab_file)
     return NULL;
 }
 
+/* merges fstab entries from both a and b, then returns the merged result.
+ * note that the caller should only manage the return pointer without
+ * doing further memory management for the two inputs, i.e. only need to
+ * frees up memory of the return value without touching a and b. */
+static struct fstab *in_place_merge(struct fstab *a, struct fstab *b)
+{
+    if (!a) return b;
+    if (!b) return a;
+
+    int total_entries = a->num_entries + b->num_entries;
+    a->recs = static_cast<struct fstab_rec *>(realloc(
+        a->recs, total_entries * (sizeof(struct fstab_rec))));
+    if (!a->recs) {
+        LERROR << __FUNCTION__ << "(): failed to allocate fstab recs";
+        // If realloc() fails the original block is left untouched;
+        // it is not freed or moved. So we have to free both a and b here.
+        fs_mgr_free_fstab(a);
+        fs_mgr_free_fstab(b);
+        return nullptr;
+    }
+
+    for (int i = a->num_entries, j = 0; i < total_entries; i++, j++) {
+        // copy the pointer directly *without* malloc and memcpy
+        a->recs[i] = b->recs[j];
+    }
+
+    // Frees up b, but don't free b->recs[X] to make sure they are
+    // accessible through a->recs[X].
+    free(b->fstab_filename);
+    free(b);
+
+    a->num_entries = total_entries;
+    return a;
+}
+
 struct fstab *fs_mgr_read_fstab(const char *fstab_path)
 {
     FILE *fstab_file;
     struct fstab *fstab;
 
     fstab_file = fopen(fstab_path, "r");
     if (!fstab_file) {
-        LERROR << "Cannot open file " << fstab_path;
-        return NULL;
+        PERROR << __FUNCTION__<< "(): cannot open file: '" << fstab_path << "'";
+        return nullptr;
     }
+
     fstab = fs_mgr_read_fstab_file(fstab_file);
     if (fstab) {
         fstab->fstab_filename = strdup(fstab_path);
+    } else {
+        LERROR << __FUNCTION__ << "(): failed to load fstab from : '" << fstab_path << "'";
     }
+
     fclose(fstab_file);
     return fstab;
 }
@@ -565,75 +604,58 @@ struct fstab *fs_mgr_read_fstab_dt()
 {
     std::string fstab_buf = read_fstab_from_dt();
     if (fstab_buf.empty()) {
-        return NULL;
+        LERROR << __FUNCTION__ << "(): failed to read fstab from dt";
+        return nullptr;
     }
 
     std::unique_ptr<FILE, decltype(&fclose)> fstab_file(
         fmemopen(static_cast<void*>(const_cast<char*>(fstab_buf.c_str())),
                  fstab_buf.length(), "r"), fclose);
     if (!fstab_file) {
-        return NULL;
+        PERROR << __FUNCTION__ << "(): failed to create a file stream for fstab dt";
+        return nullptr;
     }
 
     struct fstab *fstab = fs_mgr_read_fstab_file(fstab_file.get());
     if (!fstab) {
-        LERROR << "failed to load fstab from kernel:" << std::endl << fstab_buf;
+        LERROR << __FUNCTION__ << "(): failed to load fstab from kernel:"
+               << std::endl << fstab_buf;
     }
 
     return fstab;
 }
 
 /* combines fstab entries passed in from device tree with
- * the ones found in /fstab.<hardware>
+ * the ones found from fstab_path
  */
-struct fstab *fs_mgr_read_fstab_default()
+struct fstab *fs_mgr_read_fstab_with_dt(const char *fstab_path)
 {
-    struct fstab *fstab = fs_mgr_read_fstab_dt();
-    std::string hw;
-    if (!fs_mgr_get_boot_config("hardware", &hw)) {
-        // if we fail to find this, return whatever was found in device tree
-        LWARNING << "failed to find device hardware name";
-        return fstab;
-    }
+    struct fstab *fstab_dt = fs_mgr_read_fstab_dt();
+    struct fstab *fstab = fs_mgr_read_fstab(fstab_path);
 
-    std::string default_fstab = FSTAB_PREFIX + hw;
-    struct fstab *f = fs_mgr_read_fstab(default_fstab.c_str());
-    if (!f) {
-        // return what we have
-        LWARNING << "failed to read fstab entries from '" << default_fstab << "'";
-        return fstab;
-    }
+    return in_place_merge(fstab_dt, fstab);
+}
 
-    // return the fstab read from file if device tree doesn't
-    // have one, other wise merge the two
-    if (!fstab) {
-        fstab = f;
-    } else {
-        int total_entries = fstab->num_entries + f->num_entries;
-        fstab->recs = static_cast<struct fstab_rec *>(realloc(
-                        fstab->recs, total_entries * (sizeof(struct fstab_rec))));
-        if (!fstab->recs) {
-            LERROR << "failed to allocate fstab recs";
-            fstab->num_entries = 0;
-            fs_mgr_free_fstab(fstab);
-            return NULL;
-        }
+/*
+ * tries to load default fstab.<hardware> file from /odm/etc, /vendor/etc
+ * or /. loads the first one found and also combines fstab entries passed
+ * in from device tree.
+ */
+struct fstab *fs_mgr_read_fstab_default()
+{
+    std::string hw;
+    std::string default_fstab;
 
-        for (int i = fstab->num_entries, j = 0; i < total_entries; i++, j++) {
-            // copy everything and *not* strdup
-            fstab->recs[i] = f->recs[j];
+    if (fs_mgr_get_boot_config("hardware", &hw)) {
+        for (const char *prefix : {"/odm/etc/fstab.","/vendor/etc/fstab.", "/fstab."}) {
+            default_fstab = prefix + hw;
+            if (access(default_fstab.c_str(), F_OK) == 0) break;
         }
-
-        // free up fstab entries read from file, but don't cleanup
-        // the strings within f->recs[X] to make sure they are accessible
-        // through fstab->recs[X].
-        free(f->fstab_filename);
-        free(f);
-
-        fstab->num_entries = total_entries;
+    } else {
+        LWARNING << __FUNCTION__ << "(): failed to find device hardware name";
     }
 
-    return fstab;
+    return fs_mgr_read_fstab_with_dt(default_fstab.c_str());
 }
 
 void fs_mgr_free_fstab(struct fstab *fstab)

fs_mgr/fs_mgr_priv.h

@@ -41,8 +41,6 @@
 #define PWARNING PLOG(WARNING) << FS_MGR_TAG
 #define PERROR   PLOG(ERROR) << FS_MGR_TAG
 
-const std::string FSTAB_PREFIX("/fstab.");
-
 __BEGIN_DECLS
 
 #define CRYPTO_TMPFS_OPTIONS "size=256m,mode=0771,uid=1000,gid=1000"

fs_mgr/include/fs_mgr.h

@@ -87,8 +87,8 @@ typedef void (*fs_mgr_verity_state_callback)(struct fstab_rec *fstab,
 
 struct fstab *fs_mgr_read_fstab_default();
 struct fstab *fs_mgr_read_fstab_dt();
-struct fstab *fs_mgr_read_fstab_file(FILE *fstab_file);
 struct fstab *fs_mgr_read_fstab(const char *fstab_path);
+struct fstab *fs_mgr_read_fstab_with_dt(const char *fstab_path);
 void fs_mgr_free_fstab(struct fstab *fstab);
 
 #define FS_MGR_MNTALL_DEV_FILE_ENCRYPTED 5

init/property_service.cpp

@@ -62,7 +62,6 @@
 using android::base::StringPrintf;
 
 #define PERSISTENT_PROPERTY_DIR  "/data/property"
-#define FSTAB_PREFIX "/fstab."
 #define RECOVERY_MOUNT_POINT "/recovery"
 
 static int persistent_properties_loaded = 0;
@@ -613,21 +612,14 @@ void load_persist_props(void) {
 }
 
 void load_recovery_id_prop() {
-    std::string ro_hardware = property_get("ro.hardware");
-    if (ro_hardware.empty()) {
-        LOG(ERROR) << "ro.hardware not set - unable to load recovery id";
+    std::unique_ptr<fstab, decltype(&fs_mgr_free_fstab)> fstab(fs_mgr_read_fstab_default(),
+                                                               fs_mgr_free_fstab);
+    if (!fstab) {
+        PLOG(ERROR) << "unable to read default fstab";
         return;
     }
-    std::string fstab_filename = FSTAB_PREFIX + ro_hardware;
 
-    std::unique_ptr<fstab, void(*)(fstab*)> tab(fs_mgr_read_fstab(fstab_filename.c_str()),
-                                                fs_mgr_free_fstab);
-    if (!tab) {
-        PLOG(ERROR) << "unable to read fstab " << fstab_filename;
-        return;
-    }
-
-    fstab_rec* rec = fs_mgr_get_entry_for_mount_point(tab.get(), RECOVERY_MOUNT_POINT);
+    fstab_rec* rec = fs_mgr_get_entry_for_mount_point(fstab.get(), RECOVERY_MOUNT_POINT);
     if (rec == NULL) {
         LOG(ERROR) << "/recovery not specified in fstab";
         return;