From c8b62cf60064b8bcbb95db6e97d1ca3931eba341 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 20 Sep 2022 20:38:27 +0200 Subject: [PATCH] shared/format-table: use enum instead of Table.empty_string All users were setting this to some static string (usually "-"), so let's simplify things by not doing strdup, but instead limiting callers to a fixed set of values. In preparation for the next commit, the function is renamed from "empty" to "replacement", because it'll be used for more than empty fields. I didn't do the whole string-table setup, because it's all used internally in one file and this way we can immediately assert if an invalid value is passed in. Some callers were (void)ing the error, others were ignoring it, and others propagating. It's nicer to remove the boilerplate. --- src/busctl/busctl.c | 4 +-- src/coredump/coredumpctl.c | 2 +- src/dissect/dissect.c | 2 +- src/hostname/hostnamectl.c | 4 +-- src/locale/localectl.c | 4 +-- src/machine/machinectl.c | 2 +- src/network/networkctl.c | 3 +-- src/shared/format-table.c | 32 +++++++++++++++++------ src/shared/format-table.h | 10 ++++++- src/sysext/sysext.c | 2 +- src/systemctl/systemctl-list-jobs.c | 2 +- src/systemctl/systemctl-list-machines.c | 2 +- src/systemctl/systemctl-list-unit-files.c | 2 +- src/systemctl/systemctl-list-units.c | 8 +++--- src/sysupdate/sysupdate.c | 2 +- src/userdb/userdbctl.c | 4 +-- 16 files changed, 51 insertions(+), 34 deletions(-) diff --git a/src/busctl/busctl.c b/src/busctl/busctl.c index 90e42a21da..f57a5d605d 100644 --- a/src/busctl/busctl.c +++ b/src/busctl/busctl.c @@ -209,9 +209,7 @@ static int list_bus_names(int argc, char **argv, void *userdata) { if (r < 0) return log_error_errno(r, "Failed to set alignment: %m"); - r = table_set_empty_string(table, "-"); - if (r < 0) - return log_error_errno(r, "Failed to set empty string: %m"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); r = table_set_sort(table, (size_t) COLUMN_NAME); if (r < 0) diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index 6557e98f0a..da0b591c48 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -822,7 +822,7 @@ static int dump_list(int argc, char **argv, void *userdata) { (void) table_set_align_percent(t, TABLE_HEADER_CELL(3), 100); (void) table_set_align_percent(t, TABLE_HEADER_CELL(7), 100); - (void) table_set_empty_string(t, "-"); + table_set_ersatz_string(t, TABLE_ERSATZ_DASH); } else pager_open(arg_pager_flags); diff --git a/src/dissect/dissect.c b/src/dissect/dissect.c index b075222ec5..67b0c42124 100644 --- a/src/dissect/dissect.c +++ b/src/dissect/dissect.c @@ -570,7 +570,7 @@ static int action_dissect(DissectedImage *m, LoopDevice *d) { if (!t) return log_oom(); - (void) table_set_empty_string(t, "-"); + table_set_ersatz_string(t, TABLE_ERSATZ_DASH); (void) table_set_align_percent(t, table_get_cell(t, 0, 7), 100); for (PartitionDesignator i = 0; i < _PARTITION_DESIGNATOR_MAX; i++) { diff --git a/src/hostname/hostnamectl.c b/src/hostname/hostnamectl.c index abaea404a7..bb014973fb 100644 --- a/src/hostname/hostnamectl.c +++ b/src/hostname/hostnamectl.c @@ -92,9 +92,7 @@ static int print_status_info(StatusInfo *i) { table_set_header(table, false); - r = table_set_empty_string(table, "(unset)"); - if (r < 0) - return log_oom(); + table_set_ersatz_string(table, TABLE_ERSATZ_UNSET); r = table_add_many(table, TABLE_STRING, "Static hostname:", diff --git a/src/locale/localectl.c b/src/locale/localectl.c index b52a56be71..c23f1fa3f6 100644 --- a/src/locale/localectl.c +++ b/src/locale/localectl.c @@ -81,9 +81,7 @@ static int print_status_info(StatusInfo *i) { table_set_header(table, false); - r = table_set_empty_string(table, "(unset)"); - if (r < 0) - return log_oom(); + table_set_ersatz_string(table, TABLE_ERSATZ_UNSET); if (!strv_isempty(kernel_locale)) { log_warning("Warning: Settings on kernel command line override system locale settings in /etc/locale.conf."); diff --git a/src/machine/machinectl.c b/src/machine/machinectl.c index d05b4101cc..39e6f18606 100644 --- a/src/machine/machinectl.c +++ b/src/machine/machinectl.c @@ -277,7 +277,7 @@ static int list_machines(int argc, char *argv[], void *userdata) { if (!table) return log_oom(); - table_set_empty_string(table, "-"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); if (!arg_full && arg_max_addresses != ALL_ADDRESSES) table_set_cell_height_max(table, arg_max_addresses); diff --git a/src/network/networkctl.c b/src/network/networkctl.c index f69cde7e4d..5189ff2280 100644 --- a/src/network/networkctl.c +++ b/src/network/networkctl.c @@ -817,8 +817,7 @@ static int list_links(int argc, char *argv[], void *userdata) { table_set_width(table, 0); table_set_header(table, arg_legend); - if (table_set_empty_string(table, "-") < 0) - return log_oom(); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); assert_se(cell = table_get_cell(table, 0, 0)); (void) table_set_minimum_width(table, cell, 3); diff --git a/src/shared/format-table.c b/src/shared/format-table.c index 47bc90b9fc..3572eb2a32 100644 --- a/src/shared/format-table.c +++ b/src/shared/format-table.c @@ -133,6 +133,8 @@ struct Table { size_t n_cells; bool header; /* Whether to show the header row? */ + TableErsatz ersatz; /* What to show when we have an empty cell or an invalid value that cannot be rendered. */ + size_t width; /* If == 0 format this as wide as necessary. If SIZE_MAX format this to console * width or less wide, but not wider. Otherwise the width to format this table in. */ size_t cell_height_max; /* Maximum number of lines per cell. (If there are more, ellipsis is shown. If SIZE_MAX then no limit is set, the default. == 0 is not allowed.) */ @@ -149,8 +151,6 @@ struct Table { size_t n_json_fields; bool *reverse_map; - - char *empty_string; }; Table *table_new_raw(size_t n_columns) { @@ -167,6 +167,7 @@ Table *table_new_raw(size_t n_columns) { .header = true, .width = SIZE_MAX, .cell_height_max = SIZE_MAX, + .ersatz = TABLE_ERSATZ_EMPTY, }; return TAKE_PTR(t); @@ -242,7 +243,6 @@ Table *table_unref(Table *t) { free(t->display_map); free(t->sort_map); free(t->reverse_map); - free(t->empty_string); for (size_t i = 0; i < t->n_json_fields; i++) free(t->json_fields[i]); @@ -1089,10 +1089,26 @@ void table_set_cell_height_max(Table *t, size_t height) { t->cell_height_max = height; } -int table_set_empty_string(Table *t, const char *empty) { +void table_set_ersatz_string(Table *t, TableErsatz ersatz) { assert(t); + assert(ersatz >= 0 && ersatz < _TABLE_ERSATZ_MAX); - return free_and_strdup(&t->empty_string, empty); + t->ersatz = ersatz; +} + +static const char* table_ersatz_string(const Table *t) { + switch (t->ersatz) { + case TABLE_ERSATZ_EMPTY: + return ""; + case TABLE_ERSATZ_DASH: + return "-"; + case TABLE_ERSATZ_UNSET: + return "(unset)"; + case TABLE_ERSATZ_NA: + return "n/a"; + default: + assert_not_reached(); + } } static int table_set_display_all(Table *t) { @@ -1397,7 +1413,7 @@ static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercas switch (d->type) { case TABLE_EMPTY: - return strempty(t->empty_string); + return table_ersatz_string(t); case TABLE_STRING: case TABLE_PATH: @@ -1418,7 +1434,7 @@ static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercas case TABLE_STRV: if (strv_isempty(d->strv)) - return strempty(t->empty_string); + return table_ersatz_string(t); d->formatted = strv_join(d->strv, "\n"); if (!d->formatted) @@ -1427,7 +1443,7 @@ static const char *table_data_format(Table *t, TableData *d, bool avoid_uppercas case TABLE_STRV_WRAPPED: { if (strv_isempty(d->strv)) - return strempty(t->empty_string); + return table_ersatz_string(t); char *buf = format_strv_width(d->strv, column_width); if (!buf) diff --git a/src/shared/format-table.h b/src/shared/format-table.h index 6f60669406..3a7c2774b6 100644 --- a/src/shared/format-table.h +++ b/src/shared/format-table.h @@ -64,6 +64,14 @@ typedef enum TableDataType { _TABLE_DATA_TYPE_INVALID = -EINVAL, } TableDataType; +typedef enum TableErsatz { + TABLE_ERSATZ_EMPTY, + TABLE_ERSATZ_DASH, + TABLE_ERSATZ_UNSET, + TABLE_ERSATZ_NA, + _TABLE_ERSATZ_MAX, +} TableErsatz; + typedef struct Table Table; typedef struct TableCell TableCell; @@ -102,7 +110,7 @@ int table_add_many_internal(Table *t, TableDataType first_type, ...); void table_set_header(Table *table, bool b); void table_set_width(Table *t, size_t width); void table_set_cell_height_max(Table *t, size_t height); -int table_set_empty_string(Table *t, const char *empty); +void table_set_ersatz_string(Table *t, TableErsatz ersatz); int table_set_display_internal(Table *t, size_t first_column, ...); #define table_set_display(...) table_set_display_internal(__VA_ARGS__, SIZE_MAX) int table_set_sort_internal(Table *t, size_t first_column, ...); diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index e45fa61640..3dd37b30ca 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -166,7 +166,7 @@ static int verb_status(int argc, char **argv, void *userdata) { if (!t) return log_oom(); - (void) table_set_empty_string(t, "-"); + table_set_ersatz_string(t, TABLE_ERSATZ_DASH); STRV_FOREACH(p, arg_hierarchies) { _cleanup_free_ char *resolved = NULL, *f = NULL, *buf = NULL; diff --git a/src/systemctl/systemctl-list-jobs.c b/src/systemctl/systemctl-list-jobs.c index f9eba23683..a752173e4e 100644 --- a/src/systemctl/systemctl-list-jobs.c +++ b/src/systemctl/systemctl-list-jobs.c @@ -83,7 +83,7 @@ static int output_jobs_list(sd_bus *bus, const struct job_info* jobs, unsigned n if (arg_full) table_set_width(table, 0); - (void) table_set_empty_string(table, "-"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); for (const struct job_info *j = jobs; j < jobs + n; j++) { if (streq(j->state, "running")) diff --git a/src/systemctl/systemctl-list-machines.c b/src/systemctl/systemctl-list-machines.c index 121d5ab9ce..4407d2598a 100644 --- a/src/systemctl/systemctl-list-machines.c +++ b/src/systemctl/systemctl-list-machines.c @@ -171,7 +171,7 @@ static int output_machines_list(struct machine_info *machine_infos, unsigned n) if (arg_full) table_set_width(table, 0); - (void) table_set_empty_string(table, "-"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); for (struct machine_info *m = machine_infos; m < machine_infos + n; m++) { _cleanup_free_ char *mname = NULL; diff --git a/src/systemctl/systemctl-list-unit-files.c b/src/systemctl/systemctl-list-unit-files.c index fa7a789b28..96b22041f4 100644 --- a/src/systemctl/systemctl-list-unit-files.c +++ b/src/systemctl/systemctl-list-unit-files.c @@ -62,7 +62,7 @@ static int output_unit_file_list(const UnitFileList *units, unsigned c) { if (arg_full) table_set_width(table, 0); - (void) table_set_empty_string(table, "-"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); for (const UnitFileList *u = units; u < units + c; u++) { const char *on_underline = NULL, *on_unit_color = NULL, *id; diff --git a/src/systemctl/systemctl-list-units.c b/src/systemctl/systemctl-list-units.c index 5a1311f5ea..6bfaf97236 100644 --- a/src/systemctl/systemctl-list-units.c +++ b/src/systemctl/systemctl-list-units.c @@ -120,7 +120,7 @@ static int output_units_list(const UnitInfo *unit_infos, size_t c) { if (arg_full) table_set_width(table, 0); - (void) table_set_empty_string(table, "-"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); for (const UnitInfo *u = unit_infos; unit_infos && (size_t) (u - unit_infos) < c; u++) { _cleanup_free_ char *j = NULL; @@ -381,7 +381,7 @@ static int output_sockets_list(struct socket_info *socket_infos, size_t cs) { if (arg_full) table_set_width(table, 0); - (void) table_set_empty_string(table, "-"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); for (struct socket_info *s = socket_infos; s < socket_infos + cs; s++) { _cleanup_free_ char *j = NULL; @@ -612,7 +612,7 @@ static int output_timers_list(struct timer_info *timer_infos, size_t n) { if (arg_full) table_set_width(table, 0); - (void) table_set_empty_string(table, "-"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); for (struct timer_info *t = timer_infos; t < timer_infos + n; t++) { _cleanup_free_ char *j = NULL, *activates = NULL; @@ -844,7 +844,7 @@ static int output_automounts_list(struct automount_info *infos, size_t n_infos) if (arg_full) table_set_width(table, 0); - (void) table_set_empty_string(table, "-"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); for (struct automount_info *info = infos; info < infos + n_infos; info++) { _cleanup_free_ char *j = NULL; diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 7dcfac1261..8a4e1b5477 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -510,7 +510,7 @@ static int context_show_version(Context *c, const char *version) { (void) table_set_align_percent(t, table_get_cell(t, 0, 6), 100); (void) table_set_align_percent(t, table_get_cell(t, 0, 7), 100); (void) table_set_align_percent(t, table_get_cell(t, 0, 8), 100); - (void) table_set_empty_string(t, "-"); + table_set_ersatz_string(t, TABLE_ERSATZ_DASH); /* Determine if the target will make use of partition/fs attributes for any of the transfers */ for (size_t n = 0; n < c->n_transfers; n++) { diff --git a/src/userdb/userdbctl.c b/src/userdb/userdbctl.c index 5e9bce3bae..6afec0cda2 100644 --- a/src/userdb/userdbctl.c +++ b/src/userdb/userdbctl.c @@ -354,7 +354,7 @@ static int display_user(int argc, char *argv[], void *userdata) { (void) table_set_align_percent(table, table_get_cell(table, 0, 3), 100); (void) table_set_align_percent(table, table_get_cell(table, 0, 4), 100); - (void) table_set_empty_string(table, "-"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); (void) table_set_sort(table, (size_t) 3, (size_t) 8); (void) table_set_display(table, (size_t) 0, (size_t) 1, (size_t) 2, (size_t) 3, (size_t) 4, (size_t) 5, (size_t) 6, (size_t) 7); } @@ -657,7 +657,7 @@ static int display_group(int argc, char *argv[], void *userdata) { return log_oom(); (void) table_set_align_percent(table, table_get_cell(table, 0, 3), 100); - (void) table_set_empty_string(table, "-"); + table_set_ersatz_string(table, TABLE_ERSATZ_DASH); (void) table_set_sort(table, (size_t) 3, (size_t) 5); (void) table_set_display(table, (size_t) 0, (size_t) 1, (size_t) 2, (size_t) 3, (size_t) 4); }