From 359bed36156fe1602285c75745806d8488cf927b Mon Sep 17 00:00:00 2001 From: Bowgo Tsai Date: Thu, 27 Apr 2017 15:44:39 +0800 Subject: [PATCH] fs_mgr: code clean up - Returns FS_MGR_MNTALL_FAIL for failure paths in fs_mgr_mount_all() - Removes the 'goto out' in fs_mgr_do_mount() as there is nothing to do in the 'out' label now. Also removes the "ret = FS_MGR_DOMNT_FAILED;" and just return FS_MGR_DOMNT_FAILED directly for the default failure path. - Changes some LERROR to PERROR Test: Use fs_mgr_do_mount() to mount /system with AVB Change-Id: I126a0124a5c9d61302f40ab9db16989500d9777e --- fs_mgr/fs_mgr.cpp | 42 ++++++++++++++++----------------------- fs_mgr/fs_mgr_avb.cpp | 2 +- fs_mgr/fs_mgr_avb_ops.cpp | 4 ++-- fs_mgr/include/fs_mgr.h | 1 + 4 files changed, 21 insertions(+), 28 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index e3d4f870c..a83d26385 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -807,7 +807,7 @@ int fs_mgr_mount_all(struct fstab *fstab, int mount_mode) FsManagerAvbUniquePtr avb_handle(nullptr); if (!fstab) { - return -1; + return FS_MGR_MNTALL_FAIL; } for (i = 0; i < fstab->num_entries; i++) { @@ -853,7 +853,7 @@ int fs_mgr_mount_all(struct fstab *fstab, int mount_mode) avb_handle = FsManagerAvbHandle::Open(extract_by_name_prefix(fstab)); if (!avb_handle) { LERROR << "Failed to open FsManagerAvbHandle"; - return -1; + return FS_MGR_MNTALL_FAIL; } } if (!avb_handle->SetUpAvb(&fstab->recs[i], true /* wait_for_verity_dev */)) { @@ -983,7 +983,7 @@ int fs_mgr_mount_all(struct fstab *fstab, int mount_mode) } if (error_count) { - return -1; + return FS_MGR_MNTALL_FAIL; } else { return encryptable; } @@ -1016,14 +1016,13 @@ int fs_mgr_do_mount(struct fstab *fstab, const char *n_name, char *n_blk_device, char *tmp_mount_point) { int i = 0; - int ret = FS_MGR_DOMNT_FAILED; int mount_errors = 0; int first_mount_errno = 0; - char *m; + char* mount_point; FsManagerAvbUniquePtr avb_handle(nullptr); if (!fstab) { - return ret; + return FS_MGR_DOMNT_FAILED; } for (i = 0; i < fstab->num_entries; i++) { @@ -1038,7 +1037,7 @@ int fs_mgr_do_mount(struct fstab *fstab, const char *n_name, char *n_blk_device, !strcmp(fstab->recs[i].fs_type, "mtd")) { LERROR << "Cannot mount filesystem of type " << fstab->recs[i].fs_type << " on " << n_blk_device; - goto out; + return FS_MGR_DOMNT_FAILED; } /* First check the filesystem if requested */ @@ -1065,7 +1064,7 @@ int fs_mgr_do_mount(struct fstab *fstab, const char *n_name, char *n_blk_device, avb_handle = FsManagerAvbHandle::Open(extract_by_name_prefix(fstab)); if (!avb_handle) { LERROR << "Failed to open FsManagerAvbHandle"; - return -1; + return FS_MGR_DOMNT_FAILED; } } if (!avb_handle->SetUpAvb(&fstab->recs[i], true /* wait_for_verity_dev */)) { @@ -1086,16 +1085,15 @@ int fs_mgr_do_mount(struct fstab *fstab, const char *n_name, char *n_blk_device, /* Now mount it where requested */ if (tmp_mount_point) { - m = tmp_mount_point; + mount_point = tmp_mount_point; } else { - m = fstab->recs[i].mount_point; + mount_point = fstab->recs[i].mount_point; } int retry_count = 2; while (retry_count-- > 0) { - if (!__mount(n_blk_device, m, &fstab->recs[i])) { - ret = 0; + if (!__mount(n_blk_device, mount_point, &fstab->recs[i])) { fs_stat &= ~FS_STAT_FULL_MOUNT_FAILED; - goto out; + return FS_MGR_DOMNT_SUCCESS; } else { if (retry_count <= 0) break; // run check_fs only once if (!first_mount_errno) first_mount_errno = errno; @@ -1107,22 +1105,16 @@ int fs_mgr_do_mount(struct fstab *fstab, const char *n_name, char *n_blk_device, } log_fs_stat(fstab->recs[i].blk_device, fs_stat); } + + // Reach here means the mount attempt fails. if (mount_errors) { - PERROR << "Cannot mount filesystem on " << n_blk_device - << " at " << m; - if (first_mount_errno == EBUSY) { - ret = FS_MGR_DOMNT_BUSY; - } else { - ret = FS_MGR_DOMNT_FAILED; - } + PERROR << "Cannot mount filesystem on " << n_blk_device << " at " << mount_point; + if (first_mount_errno == EBUSY) return FS_MGR_DOMNT_BUSY; } else { /* We didn't find a match, say so and return an error */ - LERROR << "Cannot find mount point " << fstab->recs[i].mount_point - << " in fstab"; + LERROR << "Cannot find mount point " << n_name << " in fstab"; } - -out: - return ret; + return FS_MGR_DOMNT_FAILED; } /* diff --git a/fs_mgr/fs_mgr_avb.cpp b/fs_mgr/fs_mgr_avb.cpp index 83bf8a778..94cea5790 100644 --- a/fs_mgr/fs_mgr_avb.cpp +++ b/fs_mgr/fs_mgr_avb.cpp @@ -190,7 +190,7 @@ class FsManagerAvbVerifier { std::unique_ptr FsManagerAvbVerifier::Create() { std::string cmdline; if (!android::base::ReadFileToString("/proc/cmdline", &cmdline)) { - LERROR << "Failed to read /proc/cmdline"; + PERROR << "Failed to read /proc/cmdline"; return nullptr; } diff --git a/fs_mgr/fs_mgr_avb_ops.cpp b/fs_mgr/fs_mgr_avb_ops.cpp index 981e9fc04..edcfd544a 100644 --- a/fs_mgr/fs_mgr_avb_ops.cpp +++ b/fs_mgr/fs_mgr_avb_ops.cpp @@ -133,13 +133,13 @@ AvbIOResult FsManagerAvbOps::ReadFromPartition(const char* partition, int64_t of if (offset < 0) { off64_t total_size = lseek64(fd, 0, SEEK_END); if (total_size == -1) { - LERROR << "Failed to lseek64 to end of the partition"; + PERROR << "Failed to lseek64 to end of the partition"; return AVB_IO_RESULT_ERROR_IO; } offset = total_size + offset; // Repositions the offset to the beginning. if (lseek64(fd, 0, SEEK_SET) == -1) { - LERROR << "Failed to lseek64 to the beginning of the partition"; + PERROR << "Failed to lseek64 to the beginning of the partition"; return AVB_IO_RESULT_ERROR_IO; } } diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h index 12db6727f..9079a43e9 100644 --- a/fs_mgr/include/fs_mgr.h +++ b/fs_mgr/include/fs_mgr.h @@ -105,6 +105,7 @@ int fs_mgr_mount_all(struct fstab *fstab, int mount_mode); #define FS_MGR_DOMNT_FAILED (-1) #define FS_MGR_DOMNT_BUSY (-2) +#define FS_MGR_DOMNT_SUCCESS 0 int fs_mgr_do_mount(struct fstab *fstab, const char *n_name, char *n_blk_device, char *tmp_mount_point);