From 1afac11f51b27e99f31ff8c8ad7cde2ffb285631 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?H=C3=A5kan=20Kvist?= Date: Wed, 1 Jun 2022 22:03:59 +0200 Subject: Remove goto statements Use of goto is highly discouraged, so remove it. Use unique_ptr where possible to ensure that memory is freed properly Bug: 234349374 Test: manual, use bootctl to get current slot set active slot _a/_b, then dump and compare gpt tables set successful boot _a/_b, then dump and compare gpt tables Change-Id: I86b30d58efda9552057cf92499b329eeef530ef8 --- boot_control.cpp | 208 ++++++++++++++++++++++++------------------------------- 1 file changed, 91 insertions(+), 117 deletions(-) diff --git a/boot_control.cpp b/boot_control.cpp index 82daba0..98f1a12 100644 --- a/boot_control.cpp +++ b/boot_control.cpp @@ -27,6 +27,7 @@ * IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. */ #include +#include #include #include #include @@ -89,42 +90,32 @@ void boot_control_init(struct boot_control_module *module) static int get_partition_attribute(char *partname, enum part_attr_type part_attr) { - struct gpt_disk *disk = NULL; uint8_t *pentry = NULL; - int retval = -1; uint8_t *attr = NULL; if (!partname) - goto error; - disk = gpt_disk_alloc(); - if (!disk) { + return -1; + std::unique_ptr disk_raii(gpt_disk_alloc(), &gpt_disk_free); + if (!disk_raii.get()) { ALOGE("%s: Failed to alloc disk struct", __func__); - goto error; + return -1; } - if (gpt_disk_get_disk_info(partname, disk)) { + if (gpt_disk_get_disk_info(partname, disk_raii.get())) { ALOGE("%s: Failed to get disk info", __func__); - goto error; + return -1; } - pentry = gpt_disk_get_pentry(disk, partname, PRIMARY_GPT); + pentry = gpt_disk_get_pentry(disk_raii.get(), partname, PRIMARY_GPT); if (!pentry) { - ALOGE("%s: pentry does not exist in disk struct", - __func__); - goto error; + ALOGE("%s: pentry does not exist in disk struct", __func__); + return -1; } attr = pentry + AB_FLAG_OFFSET; if (part_attr == ATTR_SLOT_ACTIVE) - retval = !!(*attr & AB_PARTITION_ATTR_SLOT_ACTIVE); + return !!(*attr & AB_PARTITION_ATTR_SLOT_ACTIVE); else if (part_attr == ATTR_BOOT_SUCCESSFUL) - retval = !!(*attr & AB_PARTITION_ATTR_BOOT_SUCCESSFUL); + return !!(*attr & AB_PARTITION_ATTR_BOOT_SUCCESSFUL); else if (part_attr == ATTR_UNBOOTABLE) - retval = !!(*attr & AB_PARTITION_ATTR_UNBOOTABLE); - else - retval = -1; - gpt_disk_free(disk); - return retval; -error: - if (disk) - gpt_disk_free(disk); - return retval; + return !!(*attr & AB_PARTITION_ATTR_UNBOOTABLE); + return -1; } //Set a particular attribute for all the partitions in a @@ -135,18 +126,17 @@ static int update_slot_attribute(const char *slot, unsigned int i = 0; char buf[PATH_MAX]; struct stat st; - struct gpt_disk *disk = NULL; uint8_t *pentry = NULL; uint8_t *pentry_bak = NULL; - int rc = -1; uint8_t *attr = NULL; uint8_t *attr_bak = NULL; + std::unique_ptr disk_raii(nullptr, &gpt_disk_free); char partName[MAX_GPT_NAME_SIZE + 1] = {0}; const char ptn_list[][MAX_GPT_NAME_SIZE] = { AB_PTN_LIST }; int slot_name_valid = 0; if (!slot) { ALOGE("%s: Invalid argument", __func__); - goto error; + return -1; } for (i = 0; slot_suffix_arr[i] != NULL; i++) { @@ -156,7 +146,7 @@ static int update_slot_attribute(const char *slot, } if (!slot_name_valid) { ALOGE("%s: Invalid slot name", __func__); - goto error; + return -1; } for (i=0; i < ARRAY_SIZE(ptn_list); i++) { memset(buf, '\0', sizeof(buf)); @@ -188,26 +178,26 @@ static int update_slot_attribute(const char *slot, "%s%s", ptn_list[i], slot); - disk = gpt_disk_alloc(); - if (!disk) { + disk_raii = std::unique_ptr( + gpt_disk_alloc(), &gpt_disk_free); + if (!disk_raii.get()) { ALOGE("%s: Failed to alloc disk struct", __func__); - goto error; + return -1; } - rc = gpt_disk_get_disk_info(partName, disk); - if (rc != 0) { + if (gpt_disk_get_disk_info(partName, disk_raii.get()) != 0) { ALOGE("%s: Failed to get disk info for %s", __func__, partName); - goto error; + return -1; } - pentry = gpt_disk_get_pentry(disk, partName, PRIMARY_GPT); - pentry_bak = gpt_disk_get_pentry(disk, partName, SECONDARY_GPT); + pentry = gpt_disk_get_pentry(disk_raii.get(), partName, PRIMARY_GPT); + pentry_bak = gpt_disk_get_pentry(disk_raii.get(), partName, SECONDARY_GPT); if (!pentry || !pentry_bak) { ALOGE("%s: Failed to get pentry/pentry_bak for %s", __func__, partName); - goto error; + return -1; } attr = pentry + AB_FLAG_OFFSET; attr_bak = pentry_bak + AB_FLAG_OFFSET; @@ -223,28 +213,23 @@ static int update_slot_attribute(const char *slot, *attr_bak = (*attr) | AB_PARTITION_ATTR_SLOT_ACTIVE; } else { ALOGE("%s: Unrecognized attr", __func__); - goto error; + return -1; } - if (gpt_disk_update_crc(disk)) { + if (gpt_disk_update_crc(disk_raii.get())) { ALOGE("%s: Failed to update crc for %s", __func__, partName); - goto error; + return -1; } - if (gpt_disk_commit(disk)) { + if (gpt_disk_commit(disk_raii.get())) { ALOGE("%s: Failed to write back entry for %s", __func__, partName); - goto error; + return -1; } - gpt_disk_free(disk); - disk = NULL; } + // Successful return 0; -error: - if (disk) - gpt_disk_free(disk); - return -1; } unsigned get_number_slots(struct boot_control_module *module) @@ -254,14 +239,14 @@ unsigned get_number_slots(struct boot_control_module *module) unsigned slot_count = 0; if (!module) { ALOGE("%s: Invalid argument", __func__); - goto error; + return 0; } dir_bootdev = opendir(BOOTDEV_DIR); if (!dir_bootdev) { ALOGE("%s: Failed to open bootdev dir (%s)", __func__, strerror(errno)); - goto error; + return 0; } while ((de = readdir(dir_bootdev))) { if (de->d_name[0] == '.') @@ -274,20 +259,20 @@ unsigned get_number_slots(struct boot_control_module *module) } closedir(dir_bootdev); return slot_count; -error: - if (dir_bootdev) - closedir(dir_bootdev); - return 0; } unsigned get_current_slot(struct boot_control_module *module) { + //The HAL spec requires that we return a number between + //0 to num_slots - 1. If something went wrong just return + //return the default slot (0). + uint32_t num_slots = 0; char bootSlotProp[PROPERTY_VALUE_MAX] = {'\0'}; unsigned i = 0; if (!module) { ALOGE("%s: Invalid argument", __func__); - goto error; + return 0; } num_slots = get_number_slots(module); if (num_slots <= 1) { @@ -298,7 +283,7 @@ unsigned get_current_slot(struct boot_control_module *module) if (!strncmp(bootSlotProp, "N/A", strlen("N/A"))) { ALOGE("%s: Unable to read boot slot property", __func__); - goto error; + return 0; } //Iterate through a list of partitons named as boot+suffix //and see which one is currently active. @@ -308,10 +293,6 @@ unsigned get_current_slot(struct boot_control_module *module) strlen(slot_suffix_arr[i]))) return i; } -error: - //The HAL spec requires that we return a number between - //0 to num_slots - 1. Since something went wrong here we - //are just going to return the default slot. return 0; } @@ -332,19 +313,21 @@ static int boot_control_check_slot_sanity(struct boot_control_module *module, int mark_boot_successful(struct boot_control_module *module) { unsigned cur_slot = 0; + int retval = 0; if (!module) { ALOGE("%s: Invalid argument", __func__); - goto error; + retval = -1; } - cur_slot = get_current_slot(module); - if (update_slot_attribute(slot_suffix_arr[cur_slot], - ATTR_BOOT_SUCCESSFUL)) { - goto error; + if (retval == 0) { + cur_slot = get_current_slot(module); + if (update_slot_attribute(slot_suffix_arr[cur_slot], + ATTR_BOOT_SUCCESSFUL)) { + retval = -1; + } } - return 0; -error: - ALOGE("%s: Failed to mark boot successful", __func__); - return -1; + if (retval == -1) + ALOGE("%s: Failed to mark boot successful", __func__); + return retval; } const char *get_suffix(struct boot_control_module *module, unsigned slot) @@ -367,18 +350,15 @@ static struct gpt_disk* boot_ctl_get_disk_info(char *partition) if (!disk) { ALOGE("%s: Failed to alloc disk", __func__); - goto error; + return NULL; } if (gpt_disk_get_disk_info(partition, disk)) { ALOGE("failed to get disk info for %s", partition); - goto error; + gpt_disk_free(disk); + return NULL; } return disk; -error: - if (disk) - gpt_disk_free(disk); - return NULL; } //The argument here is a vector of partition names(including the slot suffix) @@ -387,7 +367,7 @@ static int boot_ctl_set_active_slot_for_partitions(vector part_list, unsigned slot) { char buf[PATH_MAX] = {0}; - struct gpt_disk *disk = NULL; + std::unique_ptr disk_raii(nullptr, &gpt_disk_free); char slotA[MAX_GPT_NAME_SIZE + 1] = {0}; char slotB[MAX_GPT_NAME_SIZE + 1] = {0}; char active_guid[TYPE_GUID_SIZE + 1] = {0}; @@ -409,7 +389,7 @@ static int boot_ctl_set_active_slot_for_partitions(vector part_list, string prefix = *partition_iterator; if (prefix.size() < (strlen(AB_SLOT_A_SUFFIX) + 1)) { ALOGE("Invalid partition name: %s", prefix.c_str()); - goto error; + return -1; } prefix.resize(prefix.size() - strlen(AB_SLOT_A_SUFFIX)); //Check if A/B versions of this ptn exist @@ -432,23 +412,25 @@ static int boot_ctl_set_active_slot_for_partitions(vector part_list, AB_SLOT_B_SUFFIX); //Get the disk containing the partitions that were passed in. //All partitions passed in must lie on the same disk. - if (!disk) { - disk = boot_ctl_get_disk_info(slotA); - if (!disk) - goto error; + if (!disk_raii.get()) { + disk_raii = std::unique_ptr( + boot_ctl_get_disk_info(slotA), &gpt_disk_free); + if (!disk_raii.get()) { + return -1; + } } //Get partition entry for slot A & B from the primary //and backup tables. - pentryA = gpt_disk_get_pentry(disk, slotA, PRIMARY_GPT); - pentryA_bak = gpt_disk_get_pentry(disk, slotA, SECONDARY_GPT); - pentryB = gpt_disk_get_pentry(disk, slotB, PRIMARY_GPT); - pentryB_bak = gpt_disk_get_pentry(disk, slotB, SECONDARY_GPT); + pentryA = gpt_disk_get_pentry(disk_raii.get(), slotA, PRIMARY_GPT); + pentryA_bak = gpt_disk_get_pentry(disk_raii.get(), slotA, SECONDARY_GPT); + pentryB = gpt_disk_get_pentry(disk_raii.get(), slotB, PRIMARY_GPT); + pentryB_bak = gpt_disk_get_pentry(disk_raii.get(), slotB, SECONDARY_GPT); if ( !pentryA || !pentryA_bak || !pentryB || !pentryB_bak) { //None of these should be NULL since we have already //checked for A & B versions earlier. ALOGE("Slot pentries for %s not found.", prefix.c_str()); - goto error; + return -1; } memset(active_guid, '\0', sizeof(active_guid)); memset(inactive_guid, '\0', sizeof(inactive_guid)); @@ -467,7 +449,7 @@ static int boot_ctl_set_active_slot_for_partitions(vector part_list, TYPE_GUID_SIZE); } else { ALOGE("Both A & B are inactive..Aborting"); - goto error; + return -1; } if (!strncmp(slot_suffix_arr[slot], AB_SLOT_A_SUFFIX, strlen(AB_SLOT_A_SUFFIX))){ @@ -492,30 +474,26 @@ static int boot_ctl_set_active_slot_for_partitions(vector part_list, } else { //Something has gone terribly terribly wrong ALOGE("%s: Unknown slot suffix!", __func__); - goto error; + return -1; } - if (disk) { - if (gpt_disk_update_crc(disk) != 0) { + if (disk_raii.get()) { + if (gpt_disk_update_crc(disk_raii.get()) != 0) { ALOGE("%s: Failed to update gpt_disk crc", __func__); - goto error; + return -1; } } } //write updated content to disk - if (disk) { - if (gpt_disk_commit(disk)) { + if (disk_raii.get()) { + if (gpt_disk_commit(disk_raii.get())) { ALOGE("Failed to commit disk entry"); - goto error; + return -1; } - gpt_disk_free(disk); } - return 0; -error: - if (disk) - gpt_disk_free(disk); - return -1; + // Successful + return 0; } unsigned get_active_boot_slot(struct boot_control_module *module) @@ -559,7 +537,7 @@ int set_active_boot_slot(struct boot_control_module *module, unsigned slot) if (boot_control_check_slot_sanity(module, slot)) { ALOGE("%s: Bad arguments", __func__); - goto error; + return -1; } //The partition list just contains prefixes(without the _a/_b) of the //partitions that support A/B. In order to get the layout we need the @@ -585,14 +563,14 @@ int set_active_boot_slot(struct boot_control_module *module, unsigned slot) if (gpt_utils_get_partition_map(ptn_vec, ptn_map)) { ALOGE("%s: Failed to get partition map", __func__); - goto error; + return -1; } for (map_iter = ptn_map.begin(); map_iter != ptn_map.end(); map_iter++){ if (map_iter->second.size() < 1) continue; if (boot_ctl_set_active_slot_for_partitions(map_iter->second, slot)) { ALOGE("%s: Failed to set active slot for partitions ", __func__);; - goto error; + return -1; } } if (is_ufs) { @@ -607,33 +585,31 @@ int set_active_boot_slot(struct boot_control_module *module, unsigned slot) } else { //Something has gone terribly terribly wrong ALOGE("%s: Unknown slot suffix!", __func__); - goto error; + return -1; } if (rc) { ALOGE("%s: Failed to switch xbl boot partition", __func__); - goto error; + return -1; } } return 0; -error: - return -1; } int set_slot_as_unbootable(struct boot_control_module *module, unsigned slot) { + int retval = 0; if (boot_control_check_slot_sanity(module, slot) != 0) { ALOGE("%s: Argument check failed", __func__); - goto error; + retval = -1; } - if (update_slot_attribute(slot_suffix_arr[slot], + if (retval == 0 && update_slot_attribute(slot_suffix_arr[slot], ATTR_UNBOOTABLE)) { - goto error; + retval = -1; } - return 0; -error: - ALOGE("%s: Failed to mark slot unbootable", __func__); - return -1; + if (retval != 0) + ALOGE("%s: Failed to mark slot unbootable", __func__); + return retval; } int is_slot_bootable(struct boot_control_module *module, unsigned slot) @@ -643,7 +619,7 @@ int is_slot_bootable(struct boot_control_module *module, unsigned slot) if (boot_control_check_slot_sanity(module, slot) != 0) { ALOGE("%s: Argument check failed", __func__); - goto error; + return -1; } snprintf(bootPartition, sizeof(bootPartition) - 1, "boot%s", @@ -651,7 +627,6 @@ int is_slot_bootable(struct boot_control_module *module, unsigned slot) attr = get_partition_attribute(bootPartition, ATTR_UNBOOTABLE); if (attr >= 0) return !attr; -error: return -1; } @@ -662,7 +637,7 @@ int is_slot_marked_successful(struct boot_control_module *module, unsigned slot) if (boot_control_check_slot_sanity(module, slot) != 0) { ALOGE("%s: Argument check failed", __func__); - goto error; + return -1; } snprintf(bootPartition, sizeof(bootPartition) - 1, @@ -670,7 +645,6 @@ int is_slot_marked_successful(struct boot_control_module *module, unsigned slot) attr = get_partition_attribute(bootPartition, ATTR_BOOT_SUCCESSFUL); if (attr >= 0) return attr; -error: return -1; } -- cgit v1.2.3