From: Claudius Heine <[email protected]> This patch contains a couple of minor changes for issues that where found by cppcheck and oclint.
- Reduction of variable scopes - Exiting functions early - Macro for unused parameters in ebgpart - Removed uncessesary default paths in switch statements - Removed dead code - Removed not needed variables and always true conditions Signed-off-by: Claudius Heine <[email protected]> --- drivers/watchdog/atom-quark.c | 5 ++-- main.c | 8 +++-- swupdate-adapter/ebgenv.c | 69 +++++++++++++++++++------------------------ tools/bg_setenv.c | 2 +- tools/bg_utils.c | 53 +++++++++++++-------------------- tools/ebgpart.c | 27 ++++++----------- tools/ebgpart.h | 4 +++ utils.c | 4 ++- 8 files changed, 76 insertions(+), 96 deletions(-) diff --git a/drivers/watchdog/atom-quark.c b/drivers/watchdog/atom-quark.c index 7b7774e..ccf2adb 100644 --- a/drivers/watchdog/atom-quark.c +++ b/drivers/watchdog/atom-quark.c @@ -54,10 +54,9 @@ static EFI_STATUS unlock_timer_regs(EFI_PCI_IO *pci_io, UINT32 wdt_base) static EFI_STATUS write_timer_regs(EFI_PCI_IO *pci_io, UINT32 wdt_base, UINT32 timer, UINT32 value) { - EFI_STATUS status; - unsigned int n; + for (unsigned int n = 0; n < 3; n++) { + EFI_STATUS status; - for (n = 0; n < 3; n++) { status = unlock_timer_regs(pci_io, wdt_base); if (EFI_ERROR(status)) return status; diff --git a/main.c b/main.c index 5aee63e..e796450 100644 --- a/main.c +++ b/main.c @@ -26,10 +26,11 @@ static EFI_STATUS probe_watchdog(EFI_LOADED_IMAGE *loaded_image, EFI_PCI_IO *pci_io, UINT16 pci_vendor_id, UINT16 pci_device_id, UINTN timeout) { - EFI_STATUS (*probe)(EFI_PCI_IO *, UINT16, UINT16, UINTN); const unsigned long *entry; for (entry = init_array_start; entry < init_array_end; entry++) { + EFI_STATUS (*probe)(EFI_PCI_IO *, UINT16, UINT16, UINTN); + probe = loaded_image->ImageBase + *entry; if (probe(pci_io, pci_vendor_id, pci_device_id, timeout) == EFI_SUCCESS) @@ -41,7 +42,7 @@ static EFI_STATUS probe_watchdog(EFI_LOADED_IMAGE *loaded_image, static EFI_STATUS scan_devices(EFI_LOADED_IMAGE *loaded_image, UINTN timeout) { - EFI_HANDLE device, devices[1000]; + EFI_HANDLE devices[1000]; UINTN count, size = sizeof(devices); EFI_PCI_IO *pci_io; EFI_STATUS status; @@ -58,7 +59,8 @@ static EFI_STATUS scan_devices(EFI_LOADED_IMAGE *loaded_image, UINTN timeout) return probe_watchdog(loaded_image, NULL, 0, 0, timeout); do { - device = devices[count - 1]; + EFI_HANDLE device = devices[count - 1]; + count--; status = uefi_call_wrapper(BS->OpenProtocol, 6, device, diff --git a/swupdate-adapter/ebgenv.c b/swupdate-adapter/ebgenv.c index 43390bc..4c767ee 100644 --- a/swupdate-adapter/ebgenv.c +++ b/swupdate-adapter/ebgenv.c @@ -110,31 +110,33 @@ int ebg_env_create_new(void) env_current = bgenv_get_latest(BGENVTYPE_FAT); - if (!ebg_new_env_created) { - if (!env_current) { - return EIO; - } - /* first time env is opened, a new one is created for update - * purpose */ - int new_rev = env_current->data->revision + 1; + if (ebg_new_env_created) + return env_current == NULL ? EIO : 0; - if (!bgenv_close(env_current)) { - return EIO; - } - env_current = bgenv_get_oldest(BGENVTYPE_FAT); - if (!env_current) { - return EIO; - } - /* zero fields */ - memset(env_current->data, 0, sizeof(BG_ENVDATA)); - /* update revision field and testing mode */ - env_current->data->revision = new_rev; - env_current->data->testing = 1; - /* set default watchdog timeout */ - env_current->data->watchdog_timeout_sec = 30; - ebg_new_env_created = true; - } - return env_current != NULL ? 0 : EIO; + if (!env_current) { + return EIO; + } + /* first time env is opened, a new one is created for update + * purpose */ + int new_rev = env_current->data->revision + 1; + + if (!bgenv_close(env_current)) { + return EIO; + } + env_current = bgenv_get_oldest(BGENVTYPE_FAT); + if (!env_current) { + return EIO; + } + /* zero fields */ + memset(env_current->data, 0, sizeof(BG_ENVDATA)); + /* update revision field and testing mode */ + env_current->data->revision = new_rev; + env_current->data->testing = 1; + /* set default watchdog timeout */ + env_current->data->watchdog_timeout_sec = 30; + ebg_new_env_created = true; + + return env_current == NULL ? EIO : 0; } int ebg_env_open_current(void) @@ -149,7 +151,7 @@ int ebg_env_open_current(void) env_current = bgenv_get_latest(BGENVTYPE_FAT); - return env_current != NULL ? 0 : EIO; + return env_current == NULL ? EIO : 0; } char *ebg_env_get(char *key) @@ -183,7 +185,6 @@ char *ebg_env_get(char *key) }; str16to8(buffer, env_current->data->kernelfile); return buffer; - break; case EBGENV_KERNELPARAMS: buffer = (char *)malloc(ENV_STRING_LENGTH); if (!buffer) { @@ -196,7 +197,6 @@ char *ebg_env_get(char *key) } str16to8(buffer, env_current->data->kernelparams); return buffer; - break; case EBGENV_WATCHDOG_TIMEOUT_SEC: if (asprintf(&buffer, "%lu", env_current->data->watchdog_timeout_sec) < 0) { @@ -208,7 +208,6 @@ char *ebg_env_get(char *key) return NULL; } return buffer; - break; case EBGENV_REVISION: if (asprintf(&buffer, "%lu", env_current->data->revision) < 0) { errno = ENOMEM; @@ -219,7 +218,6 @@ char *ebg_env_get(char *key) return NULL; } return buffer; - break; case EBGENV_BOOT_ONCE: if (asprintf(&buffer, "%lu", env_current->data->boot_once) < 0) { @@ -231,7 +229,6 @@ char *ebg_env_get(char *key) return NULL; } return buffer; - break; case EBGENV_TESTING: if (asprintf(&buffer, "%lu", env_current->data->testing) < 0) { errno = ENOMEM; @@ -242,7 +239,6 @@ char *ebg_env_get(char *key) return NULL; } return buffer; - break; default: errno = EINVAL; return NULL; @@ -324,18 +320,16 @@ int ebg_env_set(char *key, char *value) break; default: return EINVAL; - break; } return 0; } bool ebg_env_isupdatesuccessful(void) { - BGENV *env; - /* find all environments with revision 0 */ for (int i = 0; i < CONFIG_PARTITION_COUNT; i++) { - env = bgenv_get_by_index(BGENVTYPE_FAT, i); + BGENV *env = bgenv_get_by_index(BGENVTYPE_FAT, i); + if (!env) { continue; } @@ -354,10 +348,9 @@ bool ebg_env_isupdatesuccessful(void) int ebg_env_clearerrorstate(void) { - BGENV *env; - for (int i = 0; i < CONFIG_PARTITION_COUNT; i++) { - env = bgenv_get_by_index(BGENVTYPE_FAT, i); + BGENV *env = bgenv_get_by_index(BGENVTYPE_FAT, i); + if (!env) { continue; } diff --git a/tools/bg_setenv.c b/tools/bg_setenv.c index 1a92748..519005e 100644 --- a/tools/bg_setenv.c +++ b/tools/bg_setenv.c @@ -299,7 +299,7 @@ int main(int argc, char **argv) } if (verbosity) { printf("Updating environment with " - "revision %d\n", + "revision %u\n", env_new->data->revision); } /* if auto-updating, copy data from current diff --git a/tools/bg_utils.c b/tools/bg_utils.c index 36ee279..e44fc40 100644 --- a/tools/bg_utils.c +++ b/tools/bg_utils.c @@ -64,25 +64,28 @@ wchar_t *str8to16(wchar_t *buffer, char *src) static char *get_mountpoint(char *devpath) { - FILE *mtab = NULL; struct mntent *part = NULL; - char *mntpoint; - - if ((mtab = setmntent("/proc/mounts", "r")) != NULL) { - while ((part = getmntent(mtab)) != NULL) { - if ((part->mnt_fsname != NULL) && - (strcmp(part->mnt_fsname, devpath)) == 0) { - if (!(mntpoint = - malloc(strlen(part->mnt_dir) + 1))) { - break; - }; - strncpy(mntpoint, part->mnt_dir, - strlen(part->mnt_dir) + 1); - return mntpoint; - } + FILE *mtab = NULL; + + if ((mtab = setmntent("/proc/mounts", "r")) == NULL) + return NULL; + + while ((part = getmntent(mtab)) != NULL) { + if ((part->mnt_fsname != NULL) && + (strcmp(part->mnt_fsname, devpath)) == 0) { + char *mntpoint; + + if (!(mntpoint = + malloc(strlen(part->mnt_dir) + 1))) { + break; + }; + strncpy(mntpoint, part->mnt_dir, + strlen(part->mnt_dir) + 1); + return mntpoint; } - endmntent(mtab); } + endmntent(mtab); + return NULL; } @@ -229,10 +232,10 @@ bool probe_config_partitions(CONFIG_PART *cfgpart) continue; } if (strncmp("/dev/mmcblk", dev->path, 11) == 0) { - snprintf(devpath, 4096, "%sp%d", dev->path, + snprintf(devpath, 4096, "%sp%u", dev->path, part->num); } else { - snprintf(devpath, 4096, "%s%d", dev->path, + snprintf(devpath, 4096, "%s%u", dev->path, part->num); } if (!cfgpart[count].devpath) { @@ -365,10 +368,6 @@ bool bgenv_init(BGENVTYPE type) } } return true; - break; - default: - return false; - break; } return false; } @@ -390,9 +389,6 @@ BGENV *bgenv_get_by_index(BGENVTYPE type, uint32_t index) handle->data = &oldenvs[index]; handle->type = type; return handle; - break; - default: - return NULL; } return NULL; } @@ -411,9 +407,6 @@ BGENV *bgenv_get_oldest(BGENVTYPE type) } } return bgenv_get_by_index(type, min_idx); - break; - default: - return NULL; } return NULL; } @@ -432,9 +425,6 @@ BGENV *bgenv_get_latest(BGENVTYPE type) } } return bgenv_get_by_index(type, max_idx); - break; - default: - return NULL; } return NULL; } @@ -458,7 +448,6 @@ bool bgenv_write(BGENV *env) return false; } return true; - break; } return false; } diff --git a/tools/ebgpart.c b/tools/ebgpart.c index 07edb05..3606428 100644 --- a/tools/ebgpart.c +++ b/tools/ebgpart.c @@ -54,24 +54,18 @@ char *type_to_name(char t) switch (t) { case MBR_TYPE_FAT12: return "fat12"; - break; case MBR_TYPE_FAT16A: case MBR_TYPE_FAT16: case MBR_TYPE_FAT16_LBA: return "fat16"; - break; case MBR_TYPE_FAT32: case MBR_TYPE_FAT32_LBA: return "fat32"; - break; case MBR_TYPE_EXTENDED_LBA: case MBR_TYPE_EXTENDED: return "extended"; - break; - default: - return "not supported"; - break; } + return "not supported"; } bool check_GPT_FAT_entry(int fd, struct EFIpartitionentry *e, @@ -140,7 +134,7 @@ void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num, PedDevice *dev) { off64_t offset; struct EFIpartitionentry e; - PedPartition *partition = NULL, *tmpp; + PedPartition *tmpp; PedFileSystemType *pfst = NULL; offset = LB_SIZE * table_LBA; @@ -181,9 +175,7 @@ void read_GPT_entries(int fd, uint64_t table_LBA, uint32_t num, PedDevice *dev) if (pfst->name) free(pfst->name); free(pfst); free(tmpp); - if (!partition) { - dev->part_list = NULL; - } + dev->part_list = NULL; continue; } @@ -203,13 +195,13 @@ void scanLogicalVolumes(int fd, off64_t extended_start_LBA, if (extended_start_LBA == 0) { extended_start_LBA = offset; } - VERBOSE(stdout, "Seeking to LBA %lld\n", offset); + VERBOSE(stdout, "Seeking to LBA %ld\n", offset); off64_t res = lseek64(fd, offset * LB_SIZE, SEEK_SET); if (res == -1) { VERBOSE(stderr, "(%s)\n", strerror(errno)); return; } - VERBOSE(stdout, "Seek returned %lld\n", res); + VERBOSE(stdout, "Seek returned %ld\n", res); if (read(fd, &next_ebr, sizeof(next_ebr)) != sizeof(next_ebr)) { VERBOSE(stderr, "Error reading next EBR (%s)\n", strerror(errno)); @@ -307,7 +299,7 @@ bool check_partition_table(PedDevice *dev) efihdr.signature[6], efihdr.signature[7]); VERBOSE(stdout, "Number of partition entries: %u\n", efihdr.partitions); - VERBOSE(stdout, "Partition Table @ LBA %llu\n", + VERBOSE(stdout, "Partition Table @ LBA %lu\n", efihdr.partitiontable_LBA); read_GPT_entries(fd, efihdr.partitiontable_LBA, efihdr.partitions, dev); @@ -406,9 +398,8 @@ void ped_device_destroy(PedDevice *d) if (d->model) free(d->model); if (d->path) free(d->path); PedPartition *p = d->part_list; - PedPartition *tmpp; while (p) { - tmpp = p; + PedPartition *tmpp = p; p = p->next; ped_partition_destroy(tmpp); } @@ -425,10 +416,9 @@ PedDevice *ped_device_get_next(const PedDevice *dev) } /* free all memory */ PedDevice *d = first_device; - PedDevice *tmpd; while (d) { - tmpd = d; + PedDevice *tmpd = d; d = d->next; ped_device_destroy(tmpd); } @@ -445,5 +435,6 @@ PedDisk *ped_disk_new(const PedDevice *dev) PedPartition *ped_disk_next_partition(const PedDisk *pd, const PedPartition *part) { + UNUSED(pd); return part->next; } diff --git a/tools/ebgpart.h b/tools/ebgpart.h index 76fee9d..83c7e58 100644 --- a/tools/ebgpart.h +++ b/tools/ebgpart.h @@ -30,6 +30,10 @@ if (verbosity) fprintf(o, __VA_ARGS__) #endif +#ifndef UNUSED +#define UNUSED(x) (void)(x) +#endif + #include <unistd.h> #include <errno.h> #include <stdio.h> diff --git a/utils.c b/utils.c index 264dea5..cdfc148 100644 --- a/utils.c +++ b/utils.c @@ -163,14 +163,16 @@ EFI_STATUS get_volumes(VOLUME_DESC **volumes, UINTN *count) EFI_STATUS close_volumes(VOLUME_DESC *volumes, UINTN count) { + EFI_STATUS result = EFI_SUCCESS; UINTN i; - EFI_STATUS status, result = EFI_SUCCESS; if (!volumes) { Print(L"Invalid parameter for closing volumes.\n"); return EFI_INVALID_PARAMETER; } for (i = 0; i < count; i++) { + EFI_STATUS status; + if (!volumes[i].root) { Print(L"Error, invalid handle for volume %d.\n", i); result = EFI_INVALID_PARAMETER; -- 2.11.0 -- You received this message because you are subscribed to the Google Groups "EFI Boot Guard" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/efibootguard-dev/20170705133619.29152-1-claudius.heine.ext%40siemens.com. For more options, visit https://groups.google.com/d/optout.
