aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTao Bao2017-09-29 19:11:13 -0500
committerTao Bao2017-10-02 13:18:13 -0500
commit3e18f2bf4004e9df2f7fcee0d2035552404ff715 (patch)
tree5419ae85b390213bbca80f905584c28dd4b2fd37
parente8ee697364c1d123dccd3249a9f9e062e40a0fb4 (diff)
downloadplatform-bootable-recovery-3e18f2bf4004e9df2f7fcee0d2035552404ff715.tar.gz
platform-bootable-recovery-3e18f2bf4004e9df2f7fcee0d2035552404ff715.tar.xz
platform-bootable-recovery-3e18f2bf4004e9df2f7fcee0d2035552404ff715.zip
roots: Fix an issue with volume_for_path().
The earlier commit in 2dfc1a38982c4052bb32bc7fc06edeadf3908fb9 unintentionally changed the behavior. It gives a different result when looking up non-existent mount points (e.g. /cache on marlin). The logic behind volume_for_path("/xyz") is unclear: - It's fine to return non-null value if it's called by ensure_path_mounted() before accessing that file "/xyz". (Just based on the function name, we're not actually having this case.) - It should return nullptr if the caller is interested in the existence of that particular mount point "/xyz". This CL renames the function to volume_for_mount_point(), which does an exact match by querying the given mount point from libfs_mgr. The former volume_for_path() has been moved down to function scope for serving ensure_path_mounted() only. Test: Build and boot into recovery on bullhead and marlin respectively. 'View recovery logs'. Test: 'Mount /system' Test: 'Apply update from ADB' Change-Id: I1a16390f57540cae08a2b8f3d439d17886975217
-rw-r--r--install.cpp2
-rw-r--r--recovery.cpp2
-rw-r--r--roots.cpp6
-rw-r--r--roots.h6
4 files changed, 11 insertions, 5 deletions
diff --git a/install.cpp b/install.cpp
index 507161c2..74d1a68b 100644
--- a/install.cpp
+++ b/install.cpp
@@ -653,7 +653,7 @@ int install_package(const std::string& path, bool* wipe_cache, const std::string
653 std::chrono::duration<double> duration = std::chrono::system_clock::now() - start; 653 std::chrono::duration<double> duration = std::chrono::system_clock::now() - start;
654 int time_total = static_cast<int>(duration.count()); 654 int time_total = static_cast<int>(duration.count());
655 655
656 bool has_cache = volume_for_path("/cache") != nullptr; 656 bool has_cache = volume_for_mount_point("/cache") != nullptr;
657 // Skip logging the uncrypt_status on devices without /cache. 657 // Skip logging the uncrypt_status on devices without /cache.
658 if (has_cache) { 658 if (has_cache) {
659 static constexpr const char* UNCRYPT_STATUS = "/cache/recovery/uncrypt_status"; 659 static constexpr const char* UNCRYPT_STATUS = "/cache/recovery/uncrypt_status";
diff --git a/recovery.cpp b/recovery.cpp
index 076b4492..4dc5b540 100644
--- a/recovery.cpp
+++ b/recovery.cpp
@@ -1396,7 +1396,7 @@ int main(int argc, char **argv) {
1396 printf("Starting recovery (pid %d) on %s", getpid(), ctime(&start)); 1396 printf("Starting recovery (pid %d) on %s", getpid(), ctime(&start));
1397 1397
1398 load_volume_table(); 1398 load_volume_table();
1399 has_cache = volume_for_path(CACHE_ROOT) != nullptr; 1399 has_cache = volume_for_mount_point(CACHE_ROOT) != nullptr;
1400 1400
1401 std::vector<std::string> args = get_args(argc, argv); 1401 std::vector<std::string> args = get_args(argc, argv);
1402 std::vector<char*> args_to_parse(args.size()); 1402 std::vector<char*> args_to_parse(args.size());
diff --git a/roots.cpp b/roots.cpp
index 02c1ecb7..c0348d71 100644
--- a/roots.cpp
+++ b/roots.cpp
@@ -69,11 +69,15 @@ void load_volume_table() {
69 printf("\n"); 69 printf("\n");
70} 70}
71 71
72Volume* volume_for_mount_point(const std::string& mount_point) {
73 return fs_mgr_get_entry_for_mount_point(fstab, mount_point);
74}
75
72// Finds the volume specified by the given path. fs_mgr_get_entry_for_mount_point() does exact match 76// Finds the volume specified by the given path. fs_mgr_get_entry_for_mount_point() does exact match
73// only, so it attempts the prefixes recursively (e.g. "/cache/recovery/last_log", 77// only, so it attempts the prefixes recursively (e.g. "/cache/recovery/last_log",
74// "/cache/recovery", "/cache", "/" for a given path of "/cache/recovery/last_log") and returns the 78// "/cache/recovery", "/cache", "/" for a given path of "/cache/recovery/last_log") and returns the
75// first match or nullptr. 79// first match or nullptr.
76Volume* volume_for_path(const char* path) { 80static Volume* volume_for_path(const char* path) {
77 if (path == nullptr || path[0] == '\0') return nullptr; 81 if (path == nullptr || path[0] == '\0') return nullptr;
78 std::string str(path); 82 std::string str(path);
79 while (true) { 83 while (true) {
diff --git a/roots.h b/roots.h
index 542f03b9..46bb77e0 100644
--- a/roots.h
+++ b/roots.h
@@ -17,13 +17,15 @@
17#ifndef RECOVERY_ROOTS_H_ 17#ifndef RECOVERY_ROOTS_H_
18#define RECOVERY_ROOTS_H_ 18#define RECOVERY_ROOTS_H_
19 19
20#include <string>
21
20typedef struct fstab_rec Volume; 22typedef struct fstab_rec Volume;
21 23
22// Load and parse volume data from /etc/recovery.fstab. 24// Load and parse volume data from /etc/recovery.fstab.
23void load_volume_table(); 25void load_volume_table();
24 26
25// Return the Volume* record for this path (or NULL). 27// Return the Volume* record for this mount point (or nullptr).
26Volume* volume_for_path(const char* path); 28Volume* volume_for_mount_point(const std::string& mount_point);
27 29
28// Make sure that the volume 'path' is on is mounted. Returns 0 on 30// Make sure that the volume 'path' is on is mounted. Returns 0 on
29// success (volume is mounted). 31// success (volume is mounted).