Discussion:
[PATCH] grub-probe: Sort lists of enums and targets
(too old to reply)
Elliott Mitchell
2018-08-31 17:35:37 UTC
Permalink
This makes it distinctly easier to find things in these tables. Both for
humans trying to add entries, and for computers trying to find entries.

Signed-off-by: Elliott Mitchell <ehem+***@drgnwing.com>
---
util/grub-probe.c | 113 ++++++++++++++++++++++++++++--------------------------
1 file changed, 59 insertions(+), 54 deletions(-)

diff --git a/util/grub-probe.c b/util/grub-probe.c
index e45dbf9e0..a3ae4b750 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -57,77 +57,78 @@
#include "progname.h"

enum {
- PRINT_FS,
- PRINT_FS_UUID,
- PRINT_FS_LABEL,
- PRINT_DRIVE,
- PRINT_DEVICE,
- PRINT_PARTMAP,
- PRINT_PARTUUID,
PRINT_ABSTRACTION,
+ PRINT_ARC_HINT,
+ PRINT_BAREMETAL_HINT,
+ PRINT_BIOS_HINT,
+ PRINT_COMPATIBILITY_HINT,
PRINT_CRYPTODISK_UUID,
+ PRINT_DEVICE,
+ PRINT_DISK,
+ PRINT_DRIVE,
+ PRINT_EFI_HINT,
+ PRINT_FS,
+ PRINT_FS_LABEL,
+ PRINT_FS_UUID,
+ PRINT_GPT_PARTTYPE,
PRINT_HINT_STR,
- PRINT_BIOS_HINT,
PRINT_IEEE1275_HINT,
- PRINT_BAREMETAL_HINT,
- PRINT_EFI_HINT,
- PRINT_ARC_HINT,
- PRINT_COMPATIBILITY_HINT,
PRINT_MSDOS_PARTTYPE,
- PRINT_GPT_PARTTYPE,
+ PRINT_PARTMAP,
+ PRINT_PARTUUID,
PRINT_ZERO_CHECK,
- PRINT_DISK
};

-static const char *targets[] =
+static const struct
{
- [PRINT_FS] = "fs",
- [PRINT_FS_UUID] = "fs_uuid",
- [PRINT_FS_LABEL] = "fs_label",
- [PRINT_DRIVE] = "drive",
- [PRINT_DEVICE] = "device",
- [PRINT_PARTMAP] = "partmap",
- [PRINT_PARTUUID] = "partuuid",
- [PRINT_ABSTRACTION] = "abstraction",
- [PRINT_CRYPTODISK_UUID] = "cryptodisk_uuid",
- [PRINT_HINT_STR] = "hints_string",
- [PRINT_BIOS_HINT] = "bios_hints",
- [PRINT_IEEE1275_HINT] = "ieee1275_hints",
- [PRINT_BAREMETAL_HINT] = "baremetal_hints",
- [PRINT_EFI_HINT] = "efi_hints",
- [PRINT_ARC_HINT] = "arc_hints",
- [PRINT_COMPATIBILITY_HINT] = "compatibility_hint",
- [PRINT_MSDOS_PARTTYPE] = "msdos_parttype",
- [PRINT_GPT_PARTTYPE] = "gpt_parttype",
- [PRINT_ZERO_CHECK] = "zero_check",
- [PRINT_DISK] = "disk",
+ const char *const name;
+ const int target;
+ } targets[] =
+ {
+ { "abstraction", PRINT_ABSTRACTION },
+ { "arc_hints", PRINT_ARC_HINT },
+ { "baremetal_hints", PRINT_BAREMETAL_HINT },
+ { "bios_hints", PRINT_BIOS_HINT },
+ { "compatibility_hint", PRINT_COMPATIBILITY_HINT },
+ { "cryptodisk_uuid", PRINT_CRYPTODISK_UUID },
+ { "device", PRINT_DEVICE },
+ { "disk", PRINT_DISK },
+ { "drive", PRINT_DRIVE },
+ { "efi_hints", PRINT_EFI_HINT },
+ { "fs", PRINT_FS },
+ { "fs_label", PRINT_FS_LABEL },
+ { "fs_uuid", PRINT_FS_UUID },
+ { "gpt_parttype", PRINT_GPT_PARTTYPE },
+ { "hints_string", PRINT_HINT_STR },
+ { "ieee1275_hints", PRINT_IEEE1275_HINT },
+ { "msdos_parttype", PRINT_MSDOS_PARTTYPE },
+ { "partmap", PRINT_PARTMAP },
+ { "partuuid", PRINT_PARTUUID },
+ { "zero_check", PRINT_ZERO_CHECK },
};

static int print = PRINT_FS;
static unsigned int argument_is_device = 0;

static char *
-get_targets_string (void)
+get_targets_string (const char **defname)
{
- char **arr = xmalloc (sizeof (targets));
int len = 0;
char *str;
char *ptr;
unsigned i;

- memcpy (arr, targets, sizeof (targets));
- qsort (arr, ARRAY_SIZE (targets), sizeof (char *), grub_qsort_strcmp);
for (i = 0; i < ARRAY_SIZE (targets); i++)
- len += grub_strlen (targets[i]) + 2;
+ len += grub_strlen (targets[i].name) + 2;
ptr = str = xmalloc (len);
for (i = 0; i < ARRAY_SIZE (targets); i++)
{
- ptr = grub_stpcpy (ptr, arr[i]);
+ ptr = grub_stpcpy (ptr, targets[i].name);
*ptr++ = ',';
*ptr++ = ' ';
+ if (defname && (targets[i].target == print)) *defname = targets[i].name;
}
ptr[-2] = '\0';
- free (arr);

return str;
}
@@ -749,9 +750,10 @@ help_filter (int key, const char *text, void *input __attribute__ ((unused)))

case 't':
{
- char *ret, *t = get_targets_string (), *def;
+ const char *defname;
+ char *ret, *t = get_targets_string (&defname), *def;

- def = xasprintf (_("[default=%s]"), targets[print]);
+ def = xasprintf (_("[default=%s]"), defname);

ret = xasprintf ("%s\n%s %s %s", _("print TARGET"),
_("available targets:"), t, def);
@@ -798,16 +800,19 @@ argp_parser (int key, char *arg, struct argp_state *state)

case 't':
{
- int i;
-
- for (i = PRINT_FS; i < ARRAY_SIZE (targets); i++)
- if (strcmp (arg, targets[i]) == 0)
- {
- print = i;
- break;
- }
- if (i == ARRAY_SIZE (targets))
- argp_usage (state);
+ unsigned int lo=0, hi = ARRAY_SIZE(targets);
+ unsigned int mid;
+ int res;
+
+ while (mid = (hi + lo) / 2, res = strcmp (arg, targets[mid].name))
+ {
+ if (res < 0) hi = mid;
+ else if (res > 0) lo = mid + 1;
+
+ if (lo == hi)
+ argp_usage (state);
+ }
+ print = targets[mid].target;
}
break;
--
2.11.0

Yeah, one /should/ always try the obvious checks before initiall sending
patches. After all, everything one writes is perfect the first time.

Err...

Uhm...


--
(\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/)
\BS ( | ehem+***@drgnwing.com PGP 87145445 | ) /
\_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Elliott Mitchell
2018-08-31 17:15:20 UTC
Permalink
This makes it distinctly easier to find things in these tables. Both for
humans trying to add entries, and for computers trying to find entries.

Signed-off-by: Elliott Mitchell <ehem+***@drgnwing.com>
---
util/grub-probe.c | 109 ++++++++++++++++++++++++++++--------------------------
1 file changed, 57 insertions(+), 52 deletions(-)

diff --git a/util/grub-probe.c b/util/grub-probe.c
index e45dbf9e0..366434343 100644
--- a/util/grub-probe.c
+++ b/util/grub-probe.c
@@ -57,77 +57,78 @@
#include "progname.h"

enum {
- PRINT_FS,
- PRINT_FS_UUID,
- PRINT_FS_LABEL,
- PRINT_DRIVE,
- PRINT_DEVICE,
- PRINT_PARTMAP,
- PRINT_PARTUUID,
PRINT_ABSTRACTION,
+ PRINT_ARC_HINT,
+ PRINT_BAREMETAL_HINT,
+ PRINT_BIOS_HINT,
+ PRINT_COMPATIBILITY_HINT,
PRINT_CRYPTODISK_UUID,
+ PRINT_DEVICE,
+ PRINT_DISK,
+ PRINT_DRIVE,
+ PRINT_EFI_HINT,
+ PRINT_FS,
+ PRINT_FS_LABEL,
+ PRINT_FS_UUID,
+ PRINT_GPT_PARTTYPE,
PRINT_HINT_STR,
- PRINT_BIOS_HINT,
PRINT_IEEE1275_HINT,
- PRINT_BAREMETAL_HINT,
- PRINT_EFI_HINT,
- PRINT_ARC_HINT,
- PRINT_COMPATIBILITY_HINT,
PRINT_MSDOS_PARTTYPE,
- PRINT_GPT_PARTTYPE,
+ PRINT_PARTMAP,
+ PRINT_PARTUUID,
PRINT_ZERO_CHECK,
- PRINT_DISK
};

-static const char *targets[] =
+static const struct
{
- [PRINT_FS] = "fs",
- [PRINT_FS_UUID] = "fs_uuid",
- [PRINT_FS_LABEL] = "fs_label",
- [PRINT_DRIVE] = "drive",
- [PRINT_DEVICE] = "device",
- [PRINT_PARTMAP] = "partmap",
- [PRINT_PARTUUID] = "partuuid",
- [PRINT_ABSTRACTION] = "abstraction",
- [PRINT_CRYPTODISK_UUID] = "cryptodisk_uuid",
- [PRINT_HINT_STR] = "hints_string",
- [PRINT_BIOS_HINT] = "bios_hints",
- [PRINT_IEEE1275_HINT] = "ieee1275_hints",
- [PRINT_BAREMETAL_HINT] = "baremetal_hints",
- [PRINT_EFI_HINT] = "efi_hints",
- [PRINT_ARC_HINT] = "arc_hints",
- [PRINT_COMPATIBILITY_HINT] = "compatibility_hint",
- [PRINT_MSDOS_PARTTYPE] = "msdos_parttype",
- [PRINT_GPT_PARTTYPE] = "gpt_parttype",
- [PRINT_ZERO_CHECK] = "zero_check",
- [PRINT_DISK] = "disk",
+ char *name;
+ enum code;
+ } targets[] =
+ {
+ { "abstraction", PRINT_ABSTRACTION },
+ { "arc_hints", PRINT_ARC_HINT },
+ { "baremetal_hints", PRINT_BAREMETAL_HINT },
+ { "bios_hints", PRINT_BIOS_HINT },
+ { "compatibility_hint", PRINT_COMPATIBILITY_HINT },
+ { "cryptodisk_uuid", PRINT_CRYPTODISK_UUID },
+ { "device", PRINT_DEVICE },
+ { "disk", PRINT_DISK },
+ { "drive", PRINT_DRIVE },
+ { "efi_hints", PRINT_EFI_HINT },
+ { "fs", PRINT_FS },
+ { "fs_label", PRINT_FS_LABEL },
+ { "fs_uuid", PRINT_FS_UUID },
+ { "gpt_parttype", PRINT_GPT_PARTTYPE },
+ { "hints_string", PRINT_HINT_STR },
+ { "ieee1275_hints", PRINT_IEEE1275_HINT },
+ { "msdos_parttype", PRINT_MSDOS_PARTTYPE },
+ { "partmap", PRINT_PARTMAP },
+ { "partuuid", PRINT_PARTUUID },
+ { "zero_check", PRINT_ZERO_CHECK },
};

static int print = PRINT_FS;
static unsigned int argument_is_device = 0;

static char *
-get_targets_string (void)
+get_targets_string (const char **default)
{
- char **arr = xmalloc (sizeof (targets));
int len = 0;
char *str;
char *ptr;
unsigned i;

- memcpy (arr, targets, sizeof (targets));
- qsort (arr, ARRAY_SIZE (targets), sizeof (char *), grub_qsort_strcmp);
for (i = 0; i < ARRAY_SIZE (targets); i++)
len += grub_strlen (targets[i]) + 2;
ptr = str = xmalloc (len);
for (i = 0; i < ARRAY_SIZE (targets); i++)
{
- ptr = grub_stpcpy (ptr, arr[i]);
+ ptr = grub_stpcpy (ptr, targets[i].name);
*ptr++ = ',';
*ptr++ = ' ';
+ if (default && (targets[i].code == print)) *default = targets[i].name;
}
ptr[-2] = '\0';
- free (arr);

return str;
}
@@ -749,7 +750,8 @@ help_filter (int key, const char *text, void *input __attribute__ ((unused)))

case 't':
{
- char *ret, *t = get_targets_string (), *def;
+ const char *def;
+ char *ret, *t = get_targets_string (&def);

def = xasprintf (_("[default=%s]"), targets[print]);

@@ -798,16 +800,19 @@ argp_parser (int key, char *arg, struct argp_state *state)

case 't':
{
- int i;
-
- for (i = PRINT_FS; i < ARRAY_SIZE (targets); i++)
- if (strcmp (arg, targets[i]) == 0)
- {
- print = i;
- break;
- }
- if (i == ARRAY_SIZE (targets))
- argp_usage (state);
+ unsigned int lo=0, hi = ARRAY_SIZE(targets);
+ unsigned int mid;
+ int res;
+
+ while (mid = (hi + lo) / 2, res = strcmp (arg, targets[mid].name))
+ {
+ if (res < 0) hi = mid;
+ else if (res > 0) lo = mid + 1;
+
+ if (lo == hi)
+ argp_usage (state);
+ }
+ print = targets[mid].code;
}
break;
--
2.11.0


--
(\___(\___(\______ --=> 8-) EHM <=-- ______/)___/)___/)
\BS ( | ehem+***@drgnwing.com PGP 87145445 | ) /
\_CS\ | _____ -O #include <stddisclaimer.h> O- _____ | / _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
Daniel Kiper
2018-09-04 10:58:34 UTC
Permalink
Post by Elliott Mitchell
This makes it distinctly easier to find things in these tables. Both for
humans trying to add entries, and for computers trying to find entries.
This patch does more then commit message says. So, please improve the
commit message and/or split the patch into logical parts.

Daniel

Continue reading on narkive:
Loading...