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.

diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c
index 07f02de..0b873fb 100644 (file)
--- 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 3288b0b..ecea4f5 100644 (file)
--- 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 de28b10..2eba72d 100644 (file)
--- 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 611a3e0..f65a050 100644 (file)
--- 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 12880f8..80ac486 100644 (file)
--- 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 24eba79..d6da8c1 100644 (file)
--- a/src/shared/exit-status.h
+++ b/src/shared/exit-status.h
@@ -3,9 +3,9 @@
 
 #include <stdbool.h>
 
+#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 e17140e..5f5245e 100644 (file)
--- 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"