diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index d2425a9d872..e7e5a6a367a 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1675,10 +1675,9 @@ static INTN config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) { assert(b); /* Order entries that have no tries left to the end of the list */ - if (a->tries_left == 0 && b->tries_left != 0) - return 1; - if (a->tries_left != 0 && b->tries_left == 0) - return -1; + r = CMP(a->tries_left == 0, b->tries_left == 0); + if (r != 0) + return r; /* If there's a sort key defined for *both* entries, then we do new-style ordering, i.e. by * sort-key/machine-id/version, with a final fallback to id. If there's no sort key for either, we do @@ -1687,8 +1686,8 @@ static INTN config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) { r = CMP(!a->sort_key, !b->sort_key); if (r != 0) /* one is old-style, one new-style */ return r; - if (a->sort_key && b->sort_key) { + if (a->sort_key && b->sort_key) { r = strcmp(a->sort_key, b->sort_key); if (r != 0) return r; @@ -1704,30 +1703,23 @@ static INTN config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) { return r; } - /* Now order by ID (the version is likely part of the ID, thus note that this might put the oldest - * version last, not first, i.e. specifying a sort key explicitly is thus generally preferable, to - * take benefit of the explicit sorting above.) */ - r = strverscmp_improved(a->id, b->id); + /* Now order by ID. The version is likely part of the ID, thus note that this will generatelly put + * the newer versions earlier. Specifying a sort key explicitly is preferable, because it gives an + * explicit sort order. */ + r = -strverscmp_improved(a->id, b->id); if (r != 0) return r; - if (a->tries_left == UINTN_MAX || - b->tries_left == UINTN_MAX) + if (a->tries_left == UINTN_MAX || b->tries_left == UINTN_MAX) return 0; /* If both items have boot counting, and otherwise are identical, put the entry with more tries left first */ - if (a->tries_left < b->tries_left) - return 1; - if (a->tries_left > b->tries_left) - return -1; + r = -CMP(a->tries_left, b->tries_left); + if (r != 0) + return r; /* If they have the same number of tries left, then let the one win which was tried fewer times so far */ - if (a->tries_done > b->tries_done) - return 1; - if (a->tries_done < b->tries_done) - return -1; - - return 0; + return CMP(a->tries_done, b->tries_done); } static UINTN config_entry_find(Config *config, const CHAR16 *needle) { diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index c3b9a6cf60e..53e079bdd70 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -154,8 +154,6 @@ static int boot_entry_load( } void boot_config_free(BootConfig *config) { - size_t i; - assert(config); free(config->default_pattern); @@ -171,7 +169,7 @@ void boot_config_free(BootConfig *config) { free(config->entry_default); free(config->entry_selected); - for (i = 0; i < config->n_entries; i++) + for (size_t i = 0; i < config->n_entries; i++) boot_entry_free(config->entries + i); free(config->entries); } @@ -256,6 +254,7 @@ static int boot_entry_compare(const BootEntry *a, const BootEntry *b) { r = CMP(!a->sort_key, !b->sort_key); if (r != 0) return r; + if (a->sort_key && b->sort_key) { r = strcmp(a->sort_key, b->sort_key); if (r != 0) @@ -270,7 +269,7 @@ static int boot_entry_compare(const BootEntry *a, const BootEntry *b) { return r; } - return strverscmp_improved(a->id, b->id); + return -strverscmp_improved(a->id, b->id); } static int boot_entries_find( @@ -418,12 +417,9 @@ static int find_sections( _cleanup_free_ struct PeSectionHeader *sections = NULL; _cleanup_free_ char *osrelease = NULL, *cmdline = NULL; - size_t i, n_sections; - struct DosFileHeader dos; - struct PeHeader pe; - uint64_t start; ssize_t n; + struct DosFileHeader dos; n = pread(fd, &dos, sizeof(dos), 0); if (n < 0) return log_error_errno(errno, "Failed read DOS header: %m"); @@ -433,7 +429,9 @@ static int find_sections( if (dos.Magic[0] != 'M' || dos.Magic[1] != 'Z') return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "DOS executable magic missing, refusing."); - start = unaligned_read_le32(&dos.ExeHeader); + uint64_t start = unaligned_read_le32(&dos.ExeHeader); + + struct PeHeader pe; n = pread(fd, &pe, sizeof(pe), start); if (n < 0) return log_error_errno(errno, "Failed to read PE header: %m"); @@ -443,7 +441,7 @@ static int find_sections( if (pe.Magic[0] != 'P' || pe.Magic[1] != 'E' || pe.Magic[2] != 0 || pe.Magic[3] != 0) return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "PE executable magic missing, refusing."); - n_sections = unaligned_read_le16(&pe.FileHeader.NumberOfSections); + size_t n_sections = unaligned_read_le16(&pe.FileHeader.NumberOfSections); if (n_sections > 96) return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "PE header has too many sections, refusing."); @@ -459,7 +457,7 @@ static int find_sections( if ((size_t) n != n_sections * sizeof(struct PeSectionHeader)) return log_error_errno(SYNTHETIC_ERRNO(EIO), "Short read while reading sections, refusing."); - for (i = 0; i < n_sections; i++) { + for (size_t i = 0; i < n_sections; i++) { _cleanup_free_ char *k = NULL; uint32_t offset, size; char **b; diff --git a/src/test/test-bootspec.c b/src/test/test-bootspec.c index 47d0f58c965..7ba44744ba6 100644 --- a/src/test/test-bootspec.c +++ b/src/test/test-bootspec.c @@ -86,9 +86,9 @@ TEST_RET(bootspec_sort) { assert_se(streq(config.entries[2].id, "c.conf")); /* The following ones have no sort key, hence order by version compared ids, lowest first */ - assert_se(streq(config.entries[3].id, "a-5.conf")); + assert_se(streq(config.entries[3].id, "b.conf")); assert_se(streq(config.entries[4].id, "a-10.conf")); - assert_se(streq(config.entries[5].id, "b.conf")); + assert_se(streq(config.entries[5].id, "a-5.conf")); return 0; }