From e396c607ffa1fbd230a90c31d6bbebaa01ab89dd Mon Sep 17 00:00:00 2001 From: Sandeep Patil Date: Fri, 24 Feb 2017 11:04:49 -0800 Subject: [PATCH 1/2] fs_mgr: consolidate DT compatible check Fixes the compatible check in fs_mgr_boot_config by consolidating the check in a single privately exported function within fs_mgr (i.e. is_dt_compatible()). b/27805372 Test: Boot sailfish w/ early mount Change-Id: Ie2d1646b81cf9eba8d16828ca8cf2c75156c294c Signed-off-by: Sandeep Patil --- fs_mgr/fs_mgr_boot_config.cpp | 11 ++--------- fs_mgr/fs_mgr_fstab.cpp | 31 ++++++++++++++++--------------- fs_mgr/fs_mgr_priv.h | 1 + 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/fs_mgr/fs_mgr_boot_config.cpp b/fs_mgr/fs_mgr_boot_config.cpp index 9decb27f3..5b2f21814 100644 --- a/fs_mgr/fs_mgr_boot_config.cpp +++ b/fs_mgr/fs_mgr_boot_config.cpp @@ -49,15 +49,8 @@ bool fs_mgr_get_boot_config(const std::string& key, std::string* out_val) { } // lastly, check the device tree - std::string file_name = kAndroidDtDir + "/compatible"; - std::string dt_value; - if (android::base::ReadFileToString(file_name, &dt_value)) { - if (dt_value != "android,firmware") { - LERROR << "Error finding compatible android DT node"; - return false; - } - - file_name = kAndroidDtDir + "/" + key; + if (is_dt_compatible()) { + std::string file_name = kAndroidDtDir + "/" + key; // DT entries terminate with '\0' but so do the properties if (android::base::ReadFileToString(file_name, out_val)) { return true; diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index f98978780..e35eaa4c2 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -295,21 +295,6 @@ static int parse_flags(char *flags, struct flag_list *fl, return f; } -static bool is_dt_compatible() { - std::string file_name = kAndroidDtDir + "/compatible"; - std::string dt_value; - if (android::base::ReadFileToString(file_name, &dt_value)) { - // trim the trailing '\0' out, otherwise the comparison - // will produce false-negatives. - dt_value.resize(dt_value.size() - 1); - if (dt_value == "android,firmware") { - return true; - } - } - - return false; -} - static bool is_dt_fstab_compatible() { std::string dt_value; std::string file_name = kAndroidDtDir + "/fstab/compatible"; @@ -398,6 +383,22 @@ static std::string read_fstab_from_dt() { return fstab; } +bool is_dt_compatible() { + std::string file_name = kAndroidDtDir + "/compatible"; + std::string dt_value; + if (android::base::ReadFileToString(file_name, &dt_value)) { + if (!dt_value.empty()) { + // trim the trailing '\0' out, otherwise the comparison + // will produce false-negatives. + dt_value.resize(dt_value.size() - 1); + if (dt_value == "android,firmware") { + return true; + } + } + } + + return false; +} struct fstab *fs_mgr_read_fstab_file(FILE *fstab_file) { diff --git a/fs_mgr/fs_mgr_priv.h b/fs_mgr/fs_mgr_priv.h index 95295d8af..3b1c56b94 100644 --- a/fs_mgr/fs_mgr_priv.h +++ b/fs_mgr/fs_mgr_priv.h @@ -117,6 +117,7 @@ __BEGIN_DECLS int fs_mgr_set_blk_ro(const char *blockdev); int fs_mgr_test_access(const char *device); int fs_mgr_update_for_slotselect(struct fstab *fstab); +bool is_dt_compatible(); __END_DECLS From 4cd9a46916ec08a2686421ac974c6d083f260fee Mon Sep 17 00:00:00 2001 From: Sandeep Patil Date: Fri, 24 Feb 2017 13:03:39 -0800 Subject: [PATCH 2/2] fs_mgr: refactor: consolidate device tree file reading in one place If Device tree values are read for comparison, they produce false negatives with std::strings due to trailing '\0'. This change consolidates the triming of trailing null into a single helper function to be used everywhere fs_mgr reads DT values for comparison or other reasons where it wishes to have the trailing null trimmed. b/27805372 Test: Boot sailfish w/ early mount /vendor Change-Id: If71efc830dc440323df764c7461867e71ed6515b Signed-off-by: Sandeep Patil --- fs_mgr/fs_mgr_fstab.cpp | 48 ++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index e35eaa4c2..5b0243d0e 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -135,6 +135,24 @@ static uint64_t parse_size(const char *arg) return size; } +/* fills 'dt_value' with the underlying device tree value string without + * the trailing '\0'. Returns true if 'dt_value' has a valid string, 'false' + * otherwise. + */ +static bool read_dt_file(const std::string& file_name, std::string* dt_value) +{ + if (android::base::ReadFileToString(file_name, dt_value)) { + if (!dt_value->empty()) { + // trim the trailing '\0' out, otherwise the comparison + // will produce false-negatives. + dt_value->resize(dt_value->size() - 1); + return true; + } + } + + return false; +} + static int parse_flags(char *flags, struct flag_list *fl, struct fs_mgr_flag_values *flag_vals, char *fs_options, int fs_options_len) @@ -298,11 +316,7 @@ static int parse_flags(char *flags, struct flag_list *fl, static bool is_dt_fstab_compatible() { std::string dt_value; std::string file_name = kAndroidDtDir + "/fstab/compatible"; - - if (android::base::ReadFileToString(file_name, &dt_value)) { - // trim the trailing '\0' out, otherwise the comparison - // will produce false-negatives. - dt_value.resize(dt_value.size() - 1); + if (read_dt_file(file_name, &dt_value)) { if (dt_value == "android,fstab") { return true; } @@ -339,41 +353,36 @@ static std::string read_fstab_from_dt() { std::string file_name; std::string value; file_name = android::base::StringPrintf("%s/%s/dev", fstabdir_name.c_str(), dp->d_name); - if (!android::base::ReadFileToString(file_name, &value)) { + if (!read_dt_file(file_name, &value)) { LERROR << "dt_fstab: Failed to find device for partition " << dp->d_name; fstab.clear(); break; } - // trim the terminating '\0' out - value.resize(value.size() - 1); fstab_entry.push_back(value); fstab_entry.push_back(android::base::StringPrintf("/%s", dp->d_name)); file_name = android::base::StringPrintf("%s/%s/type", fstabdir_name.c_str(), dp->d_name); - if (!android::base::ReadFileToString(file_name, &value)) { + if (!read_dt_file(file_name, &value)) { LERROR << "dt_fstab: Failed to find type for partition " << dp->d_name; fstab.clear(); break; } - value.resize(value.size() - 1); fstab_entry.push_back(value); file_name = android::base::StringPrintf("%s/%s/mnt_flags", fstabdir_name.c_str(), dp->d_name); - if (!android::base::ReadFileToString(file_name, &value)) { + if (!read_dt_file(file_name, &value)) { LERROR << "dt_fstab: Failed to find type for partition " << dp->d_name; fstab.clear(); break; } - value.resize(value.size() - 1); fstab_entry.push_back(value); file_name = android::base::StringPrintf("%s/%s/fsmgr_flags", fstabdir_name.c_str(), dp->d_name); - if (!android::base::ReadFileToString(file_name, &value)) { + if (!read_dt_file(file_name, &value)) { LERROR << "dt_fstab: Failed to find type for partition " << dp->d_name; fstab.clear(); break; } - value.resize(value.size() - 1); fstab_entry.push_back(value); fstab += android::base::Join(fstab_entry, " "); @@ -386,14 +395,9 @@ static std::string read_fstab_from_dt() { bool is_dt_compatible() { std::string file_name = kAndroidDtDir + "/compatible"; std::string dt_value; - if (android::base::ReadFileToString(file_name, &dt_value)) { - if (!dt_value.empty()) { - // trim the trailing '\0' out, otherwise the comparison - // will produce false-negatives. - dt_value.resize(dt_value.size() - 1); - if (dt_value == "android,firmware") { - return true; - } + if (read_dt_file(file_name, &dt_value)) { + if (dt_value == "android,firmware") { + return true; } }