From 23d5dd168724fe60c7b00d78f49563a6be05627d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 28 Jul 2019 11:14:46 +0200 Subject: shared/exit-status: use Bitmap instead of Sets I opted to embed the Bitmap structure directly in the ExitStatusSet. This means that memory usage is a bit higher for units which don't define this setting: Service changes: /* size: 2720, cachelines: 43, members: 73 */ /* sum members: 2680, holes: 9, sum holes: 39 */ /* sum bitfield members: 7 bits, bit holes: 1, sum bit holes: 1 bits */ /* last cacheline: 32 bytes */ /* size: 2816, cachelines: 44, members: 73 */ /* sum members: 2776, holes: 9, sum holes: 39 */ /* sum bitfield members: 7 bits, bit holes: 1, sum bit holes: 1 bits */ But this way the code is simpler and we do less pointer chasing. --- src/core/dbus-service.c | 32 +++++++--------------- src/core/load-fragment.c | 12 +++----- src/shared/bitmap.c | 6 ---- src/shared/bitmap.h | 6 +++- src/shared/exit-status.c | 27 ++++++++---------- src/shared/exit-status.h | 12 ++++---- .../tty-ask-password-agent.c | 1 + 7 files changed, 38 insertions(+), 58 deletions(-) diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 07f02deed6..0b873fb486 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -39,9 +39,9 @@ static int property_get_exit_status_set( void *userdata, sd_bus_error *error) { - ExitStatusSet *status_set = userdata; + const ExitStatusSet *status_set = userdata; + unsigned n; Iterator i; - void *id; int r; assert(bus); @@ -56,13 +56,10 @@ static int property_get_exit_status_set( if (r < 0) return r; - SET_FOREACH(id, status_set->status, i) { - int32_t val = PTR_TO_INT(id); + BITMAP_FOREACH(n, &status_set->status, i) { + assert(n < 256); - if (val < 0 || val > 255) - continue; - - r = sd_bus_message_append_basic(reply, 'i', &val); + r = sd_bus_message_append_basic(reply, 'i', &n); if (r < 0) return r; } @@ -75,15 +72,14 @@ static int property_get_exit_status_set( if (r < 0) return r; - SET_FOREACH(id, status_set->signal, i) { - int32_t val = PTR_TO_INT(id); + BITMAP_FOREACH(n, &status_set->signal, i) { const char *str; - str = signal_to_string((int) val); + str = signal_to_string(n); if (!str) continue; - r = sd_bus_message_append_basic(reply, 'i', &val); + r = sd_bus_message_append_basic(reply, 'i', &n); if (r < 0) return r; } @@ -196,11 +192,7 @@ static int bus_set_transient_exit_status( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid status code in %s: %"PRIi32, name, status[i]); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - r = set_ensure_allocated(&status_set->status, NULL); - if (r < 0) - return r; - - r = set_put(status_set->status, INT_TO_PTR((int) status[i])); + r = bitmap_set(&status_set->status, status[i]); if (r < 0) return r; @@ -216,11 +208,7 @@ static int bus_set_transient_exit_status( return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid signal in %s: %"PRIi32, name, signal[i]); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - r = set_ensure_allocated(&status_set->signal, NULL); - if (r < 0) - return r; - - r = set_put(status_set->signal, INT_TO_PTR((int) signal[i])); + r = bitmap_set(&status_set->signal, signal[i]); if (r < 0) return r; diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 3288b0b838..ecea4f526a 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -3937,7 +3937,7 @@ int config_parse_set_status( FOREACH_WORD(word, l, rvalue, state) { _cleanup_free_ char *temp; int val; - Set **set; + Bitmap *bitmap; temp = strndup(word, l); if (!temp) @@ -3951,20 +3951,16 @@ int config_parse_set_status( log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse value, ignoring: %s", word); continue; } - set = &status_set->signal; + bitmap = &status_set->signal; } else { if (val < 0 || val > 255) { log_syntax(unit, LOG_ERR, filename, line, 0, "Value %d is outside range 0-255, ignoring", val); continue; } - set = &status_set->status; + bitmap = &status_set->status; } - r = set_ensure_allocated(set, NULL); - if (r < 0) - return log_oom(); - - r = set_put(*set, INT_TO_PTR(val)); + r = bitmap_set(bitmap, val); if (r < 0) return log_oom(); } diff --git a/src/shared/bitmap.c b/src/shared/bitmap.c index de28b1055a..2eba72dd59 100644 --- a/src/shared/bitmap.c +++ b/src/shared/bitmap.c @@ -12,12 +12,6 @@ #include "macro.h" #include "memory-util.h" -struct Bitmap { - uint64_t *bitmaps; - size_t n_bitmaps; - size_t bitmaps_allocated; -}; - /* Bitmaps are only meant to store relatively small numbers * (corresponding to, say, an enum), so it is ok to limit * the max entry. 64k should be plenty. */ diff --git a/src/shared/bitmap.h b/src/shared/bitmap.h index 611a3e0e9d..f65a050584 100644 --- a/src/shared/bitmap.h +++ b/src/shared/bitmap.h @@ -6,7 +6,11 @@ #include "hashmap.h" #include "macro.h" -typedef struct Bitmap Bitmap; +typedef struct Bitmap { + uint64_t *bitmaps; + size_t n_bitmaps; + size_t bitmaps_allocated; +} Bitmap; Bitmap *bitmap_new(void); Bitmap *bitmap_copy(Bitmap *b); diff --git a/src/shared/exit-status.c b/src/shared/exit-status.c index 12880f805e..80ac4868cb 100644 --- a/src/shared/exit-status.c +++ b/src/shared/exit-status.c @@ -134,18 +134,19 @@ int exit_status_from_string(const char *s) { return val; } -bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success_status) { +bool is_clean_exit(int code, int status, ExitClean clean, const ExitStatusSet *success_status) { if (code == CLD_EXITED) return status == 0 || (success_status && - set_contains(success_status->status, INT_TO_PTR(status))); + bitmap_isset(&success_status->status, status)); - /* If a daemon does not implement handlers for some of the signals that's not considered an unclean shutdown */ + /* If a daemon does not implement handlers for some of the signals, we do not consider this an + unclean shutdown */ if (code == CLD_KILLED) return (clean == EXIT_CLEAN_DAEMON && IN_SET(status, SIGHUP, SIGINT, SIGTERM, SIGPIPE)) || (success_status && - set_contains(success_status->signal, INT_TO_PTR(status))); + bitmap_isset(&success_status->signal, status)); return false; } @@ -153,26 +154,22 @@ bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success void exit_status_set_free(ExitStatusSet *x) { assert(x); - x->status = set_free(x->status); - x->signal = set_free(x->signal); + bitmap_clear(&x->status); + bitmap_clear(&x->signal); } -bool exit_status_set_is_empty(ExitStatusSet *x) { +bool exit_status_set_is_empty(const ExitStatusSet *x) { if (!x) return true; - return set_isempty(x->status) && set_isempty(x->signal); + return bitmap_isclear(&x->status) && bitmap_isclear(&x->signal); } -bool exit_status_set_test(ExitStatusSet *x, int code, int status) { - - if (exit_status_set_is_empty(x)) - return false; - - if (code == CLD_EXITED && set_contains(x->status, INT_TO_PTR(status))) +bool exit_status_set_test(const ExitStatusSet *x, int code, int status) { + if (code == CLD_EXITED && bitmap_isset(&x->status, status)) return true; - if (IN_SET(code, CLD_KILLED, CLD_DUMPED) && set_contains(x->signal, INT_TO_PTR(status))) + if (IN_SET(code, CLD_KILLED, CLD_DUMPED) && bitmap_isset(&x->signal, status)) return true; return false; diff --git a/src/shared/exit-status.h b/src/shared/exit-status.h index 24eba79f56..d6da8c19b9 100644 --- a/src/shared/exit-status.h +++ b/src/shared/exit-status.h @@ -3,9 +3,9 @@ #include +#include "bitmap.h" #include "hashmap.h" #include "macro.h" -#include "set.h" /* This defines pretty names for the LSB 'start' verb exit codes. Note that they shouldn't be confused with * the LSB 'status' verb exit codes which are defined very differently. For details see: @@ -83,8 +83,8 @@ typedef enum ExitStatusClass { } ExitStatusClass; typedef struct ExitStatusSet { - Set *status; - Set *signal; + Bitmap status; + Bitmap signal; } ExitStatusSet; const char* exit_status_to_string(int code, ExitStatusClass class) _const_; @@ -103,8 +103,8 @@ typedef enum ExitClean { EXIT_CLEAN_COMMAND, } ExitClean; -bool is_clean_exit(int code, int status, ExitClean clean, ExitStatusSet *success_status); +bool is_clean_exit(int code, int status, ExitClean clean, const ExitStatusSet *success_status); void exit_status_set_free(ExitStatusSet *x); -bool exit_status_set_is_empty(ExitStatusSet *x); -bool exit_status_set_test(ExitStatusSet *x, int code, int status); +bool exit_status_set_is_empty(const ExitStatusSet *x); +bool exit_status_set_test(const ExitStatusSet *x, int code, int status); diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index e17140ea0c..5f5245e48a 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -39,6 +39,7 @@ #include "plymouth-util.h" #include "pretty-print.h" #include "process-util.h" +#include "set.h" #include "signal-util.h" #include "socket-util.h" #include "string-util.h" -- cgit v1.2.1