SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

install: make InstallChange enum type a proper type

Browse Source 
We can just make this an enum, as long as we ensure it has enough range,
which we can do by adding -ERRNO_MAX as one possible value (at least on
GNU C). We already do that at multiple other places, so let's do this
here too.
This commit is contained in:
Lennart Poettering
2022-10-14 11:40:40 +02:00
parent 4554c178bf
commit f8662fee2f
8 changed files with 103 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/shared/bus-unit-util.c
+8 -6
View File
@@ -2682,16 +2682,18 @@ int bus_deserialize_and_dump_unit_file_changes(sd_bus_message *m, bool quiet, In
return bus_log_parse_error(r);
while ((r = sd_bus_message_read(m, "(sss)", &type, &path, &source)) > 0) {
/* We expect only "success" changes to be sent over the bus.
Hence, reject anything negative. */
int ch = install_change_from_string(type);
if (ch < 0) {
log_notice_errno(ch, "Manager reported unknown change type \"%s\" for path \"%s\", ignoring.",
InstallChangeType t;
/* We expect only "success" changes to be sent over the bus. Hence, reject anything
* negative. */
t = install_change_type_from_string(type);
if (t < 0) {
log_notice_errno(t, "Manager reported unknown change type \"%s\" for path \"%s\", ignoring.",
type, path);
continue;
}
r = install_changes_add(changes, n_changes, ch, path, source);
r = install_changes_add(changes, n_changes, t, path, source);
if (r < 0)
return r;
}
src/shared/install.c
+21 -25
View File
@@ -268,7 +268,7 @@ static const char* config_path_from_flags(const LookupPaths *lp, UnitFileFlags f
int install_changes_add(
InstallChange **changes,
size_t *n_changes,
int change_or_errno, /* INSTALL_CHANGE_SYMLINK, _UNLINK, _IS_MASKED, _IS_DANGLING, … if positive or errno if negative */
InstallChangeType type, /* INSTALL_CHANGE_SYMLINK, _UNLINK, _IS_MASKED, _IS_DANGLING, … if positive or errno if negative */
const char *path,
const char *source) {
@@ -276,11 +276,7 @@ int install_changes_add(
InstallChange *c;
assert(!changes == !n_changes);
if (change_or_errno >= 0)
assert(change_or_errno < _INSTALL_CHANGE_MAX);
else
assert(change_or_errno >= -ERRNO_MAX);
assert(INSTALL_CHANGE_TYPE_VALID(type));
if (!changes)
return 0;
@@ -307,7 +303,7 @@ int install_changes_add(
}
c[(*n_changes)++] = (InstallChange) {
.change_or_errno = change_or_errno,
.type = type,
.path = TAKE_PTR(p),
.source = TAKE_PTR(s),
};
@@ -334,9 +330,9 @@ void install_changes_dump(int r, const char *verb, const InstallChange *changes,
assert(verb || r >= 0);
for (size_t i = 0; i < n_changes; i++) {
assert(verb || changes[i].change_or_errno >= 0);
assert(verb || changes[i].type >= 0);
switch (changes[i].change_or_errno) {
switch (changes[i].type) {
case INSTALL_CHANGE_SYMLINK:
if (!quiet)
log_info("Created symlink %s %s %s.",
@@ -373,58 +369,58 @@ void install_changes_dump(int r, const char *verb, const InstallChange *changes,
break;
case -EEXIST:
if (changes[i].source)
err = log_error_errno(changes[i].change_or_errno,
err = log_error_errno(changes[i].type,
"Failed to %s unit, file \"%s\" already exists and is a symlink to \"%s\".",
verb, changes[i].path, changes[i].source);
else
err = log_error_errno(changes[i].change_or_errno,
err = log_error_errno(changes[i].type,
"Failed to %s unit, file \"%s\" already exists.",
verb, changes[i].path);
break;
case -ERFKILL:
err = log_error_errno(changes[i].change_or_errno, "Failed to %s unit, unit %s is masked.",
err = log_error_errno(changes[i].type, "Failed to %s unit, unit %s is masked.",
verb, changes[i].path);
break;
case -EADDRNOTAVAIL:
err = log_error_errno(changes[i].change_or_errno, "Failed to %s unit, unit %s is transient or generated.",
err = log_error_errno(changes[i].type, "Failed to %s unit, unit %s is transient or generated.",
verb, changes[i].path);
break;
case -EBADSLT:
err = log_error_errno(changes[i].change_or_errno, "Failed to %s unit, invalid specifier in \"%s\".",
err = log_error_errno(changes[i].type, "Failed to %s unit, invalid specifier in \"%s\".",
verb, changes[i].path);
break;
case -EIDRM:
err = log_error_errno(changes[i].change_or_errno, "Failed to %s %s, destination unit %s is a non-template unit.",
err = log_error_errno(changes[i].type, "Failed to %s %s, destination unit %s is a non-template unit.",
verb, changes[i].source, changes[i].path);
break;
case -EUCLEAN:
err = log_error_errno(changes[i].change_or_errno,
err = log_error_errno(changes[i].type,
"Failed to %s unit, \"%s\" is not a valid unit name.",
verb, changes[i].path);
break;
case -ELOOP:
err = log_error_errno(changes[i].change_or_errno, "Failed to %s unit, refusing to operate on linked unit file %s.",
err = log_error_errno(changes[i].type, "Failed to %s unit, refusing to operate on linked unit file %s.",
verb, changes[i].path);
break;
case -EXDEV:
if (changes[i].source)
err = log_error_errno(changes[i].change_or_errno, "Failed to %s unit, cannot alias %s as %s.",
err = log_error_errno(changes[i].type, "Failed to %s unit, cannot alias %s as %s.",
verb, changes[i].source, changes[i].path);
else
err = log_error_errno(changes[i].change_or_errno, "Failed to %s unit, invalid unit reference \"%s\".",
err = log_error_errno(changes[i].type, "Failed to %s unit, invalid unit reference \"%s\".",
verb, changes[i].path);
break;
case -ENOENT:
err = log_error_errno(changes[i].change_or_errno, "Failed to %s unit, unit %s does not exist.",
err = log_error_errno(changes[i].type, "Failed to %s unit, unit %s does not exist.",
verb, changes[i].path);
break;
case -EUNATCH:
err = log_error_errno(changes[i].change_or_errno, "Failed to %s unit, cannot resolve specifiers in \"%s\".",
err = log_error_errno(changes[i].type, "Failed to %s unit, cannot resolve specifiers in \"%s\".",
verb, changes[i].path);
break;
default:
assert(changes[i].change_or_errno < 0);
err = log_error_errno(changes[i].change_or_errno, "Failed to %s unit, file \"%s\": %m",
assert(changes[i].type < 0);
err = log_error_errno(changes[i].type, "Failed to %s unit, file \"%s\": %m",
verb, changes[i].path);
}
}
@@ -3666,7 +3662,7 @@ static const char* const unit_file_state_table[_UNIT_FILE_STATE_MAX] = {
DEFINE_STRING_TABLE_LOOKUP(unit_file_state, UnitFileState);
static const char* const install_change_table[_INSTALL_CHANGE_MAX] = {
static const char* const install_change_type_table[_INSTALL_CHANGE_TYPE_MAX] = {
[INSTALL_CHANGE_SYMLINK] = "symlink",
[INSTALL_CHANGE_UNLINK] = "unlink",
[INSTALL_CHANGE_IS_MASKED] = "masked",
@@ -3676,7 +3672,7 @@ static const char* const install_change_table[_INSTALL_CHANGE_MAX] = {
[INSTALL_CHANGE_AUXILIARY_FAILED] = "auxiliary unit failed",
};
DEFINE_STRING_TABLE_LOOKUP(install_change, int);
DEFINE_STRING_TABLE_LOOKUP(install_change_type, InstallChangeType);
static const char* const unit_file_preset_mode_table[_UNIT_FILE_PRESET_MAX] = {
[UNIT_FILE_PRESET_FULL] = "full",
src/shared/install.h
+13 -10
View File
@@ -26,9 +26,7 @@ enum UnitFilePresetMode {
_UNIT_FILE_PRESET_INVALID = -EINVAL,
};
/* This enum type is anonymous, since we usually store it in an 'int', as we overload it with negative errno
* values. */
enum {
enum InstallChangeType {
INSTALL_CHANGE_SYMLINK,
INSTALL_CHANGE_UNLINK,
INSTALL_CHANGE_IS_MASKED,
@@ -36,10 +34,15 @@ enum {
INSTALL_CHANGE_IS_DANGLING,
INSTALL_CHANGE_DESTINATION_NOT_PRESENT,
INSTALL_CHANGE_AUXILIARY_FAILED,
_INSTALL_CHANGE_MAX,
_INSTALL_CHANGE_TYPE_MAX,
_INSTALL_CHANGE_INVALID = -EINVAL,
_INSTALL_CHANGE_ERRNO_MAX = -ERRNO_MAX, /* Ensure this type covers the whole negative errno range */
};
static inline bool INSTALL_CHANGE_TYPE_VALID(InstallChangeType t) {
return t >= _INSTALL_CHANGE_ERRNO_MAX && t < _INSTALL_CHANGE_TYPE_MAX;
}
enum UnitFileFlags {
UNIT_FILE_RUNTIME = 1 << 0, /* Public API via DBUS, do not change */
UNIT_FILE_FORCE = 1 << 1, /* Public API via DBUS, do not change */
@@ -49,20 +52,20 @@ enum UnitFileFlags {
_UNIT_FILE_FLAGS_MASK_PUBLIC = UNIT_FILE_RUNTIME|UNIT_FILE_PORTABLE|UNIT_FILE_FORCE,
};
/* change_or_errno can be either one of the INSTALL_CHANGE_SYMLINK, INSTALL_CHANGE_UNLINK, … listed above, or
* a negative errno value.
/* type can be either one of the INSTALL_CHANGE_SYMLINK, INSTALL_CHANGE_UNLINK, … listed above, or a negative
* errno value.
*
* If source is specified, it should be the contents of the path symlink. In case of an error, source should
* be the existing symlink contents or NULL. */
struct InstallChange {
int change_or_errno; /* INSTALL_CHANGE_SYMLINK, … if positive, errno if negative */
int type; /* INSTALL_CHANGE_SYMLINK, … if positive, errno if negative */
char *path;
char *source;
};
static inline bool install_changes_have_modification(const InstallChange* changes, size_t n_changes) {
for (size_t i = 0; i < n_changes; i++)
if (IN_SET(changes[i].change_or_errno, INSTALL_CHANGE_SYMLINK, INSTALL_CHANGE_UNLINK))
if (IN_SET(changes[i].type, INSTALL_CHANGE_SYMLINK, INSTALL_CHANGE_UNLINK))
return true;
return false;
}
@@ -220,8 +223,8 @@ const char *unit_file_state_to_string(UnitFileState s) _const_;
UnitFileState unit_file_state_from_string(const char *s) _pure_;
/* from_string conversion is unreliable because of the overlap between -EPERM and -1 for error. */
const char *install_change_to_string(int s) _const_;
int install_change_from_string(const char *s) _pure_;
const char *install_change_type_to_string(InstallChangeType t) _const_;
int install_change_type_from_string(const char *s) _pure_;
const char *unit_file_preset_mode_to_string(UnitFilePresetMode m) _const_;
UnitFilePresetMode unit_file_preset_mode_from_string(const char *s) _pure_;