efi: efi_get_boot_options() should already sort the entries, the random order in the efivars fs is probably not useful

This also introduces a new FOREACH_DIRENT macro and makes use of it.

3 files changed, 36 insertions, 8 deletions

diff --git a/src/shared/efivars.c b/src/shared/efivars.c
index 064746a9a8..840e4e0c35 100644
--- a/src/shared/efivars.c
+++ b/src/shared/efivars.c
@@ -303,11 +303,22 @@ static int boot_id_hex(const char s[4]) {
         return id;
 }
 
+static int cmp_uint16(const void *_a, const void *_b) {
+        const uint16_t *a = _a, *b = _b;
+
+        if (*a < *b)
+                return -1;
+        if (*a > *b)
+                return 1;
+
+        return 0;
+}
+
 int efi_get_boot_options(uint16_t **options) {
         _cleanup_closedir_ DIR *dir = NULL;
         struct dirent *de;
         uint16_t *list = NULL;
-        int count = 0;
+        int count = 0, r;
 
         assert(options);
 
@@ -315,7 +326,7 @@ int efi_get_boot_options(uint16_t **options) {
         if (!dir)
                 return -errno;
 
-        while ((de = readdir(dir))) {
+        FOREACH_DIRENT(de, dir, r = -errno; goto fail) {
                 int id;
                 uint16_t *t;
 
@@ -334,17 +345,22 @@ int efi_get_boot_options(uint16_t **options) {
 
                 t = realloc(list, (count + 1) * sizeof(uint16_t));
                 if (!t) {
-                        free(list);
-                        return -ENOMEM;
+                        r = -ENOMEM;
+                        goto fail;
                 }
 
                 list = t;
                 list[count ++] = id;
-
         }
 
+        qsort(list, count, sizeof(uint16_t), cmp_uint16);
+
         *options = list;
         return count;
+
+fail:
+        free(list);
+        return r;
 }
 
 static int read_usec(sd_id128_t vendor, const char *name, usec_t *u) {
diff --git a/src/shared/util.c b/src/shared/util.c
index d754c836f2..4f0b652f4f 100644
--- a/src/shared/util.c
+++ b/src/shared/util.c
@@ -1077,6 +1077,7 @@ int get_process_exe(pid_t pid, char **name) {
 static int get_process_id(pid_t pid, const char *field, uid_t *uid) {
         _cleanup_fclose_ FILE *f = NULL;
         _cleanup_free_ char *p = NULL;
+        char line[LINE_MAX];
 
         assert(field);
         assert(uid);
@@ -1091,7 +1092,7 @@ static int get_process_id(pid_t pid, const char *field, uid_t *uid) {
         if (!f)
                 return -errno;
 
-        FOREACH_LINE(f, line, return -errno) {
+        FOREACH_LINE(line, f, return -errno) {
                 char *l;
 
                 l = strstrip(line);
diff --git a/src/shared/util.h b/src/shared/util.h
index 3ad90ddce4..fcb0d9af17 100644
--- a/src/shared/util.h
+++ b/src/shared/util.h
@@ -573,11 +573,22 @@ int on_ac_power(void);
 int search_and_fopen(const char *path, const char *mode, const char **search, FILE **_f);
 int search_and_fopen_nulstr(const char *path, const char *mode, const char *search, FILE **_f);
 
-#define FOREACH_LINE(f, line, on_error)                         \
-        for (char line[LINE_MAX]; !feof(f); )                   \
+#define FOREACH_LINE(line, f, on_error)                         \
+        for (;;)                                                \
                 if (!fgets(line, sizeof(line), f)) {            \
                         if (ferror(f)) {                        \
                                 on_error;                       \
                         }                                       \
                         break;                                  \
                 } else
+
+#define FOREACH_DIRENT(de, d, on_error)                                 \
+        for (errno = 0, de = readdir(d);; errno = 0, de = readdir(d))   \
+                if (!de) {                                              \
+                        if (errno != 0) {                               \
+                                on_error;                               \
+                        }                                               \
+                        break;                                          \
+                } else if (ignore_file((de)->d_name))                   \
+                        continue;                                       \
+                else