udev-rules: add precise information to rule failure logs

It is pretty hard to figure out what the problem actually is, esp. when the rule
is long.

On my machine:
systemd[1]: Starting udev Kernel Device Manager...
systemd-udevd[217399]: /usr/lib/udev/rules.d/11-dm-lvm.rules:40 Invalid value for OPTIONS key, ignoring: 'event_timeout=180'
systemd-udevd[217399]: /usr/lib/udev/rules.d/11-dm-lvm.rules:40 The line takes no effect, ignoring.
systemd-udevd[217399]: /etc/udev/rules.d/60-ipath.rules:4 Invalid value "kcopy/%02n" for NAME (char 7: invalid substitution type), ignoring, but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/65-md-incremental.rules:28 Invalid value "/sbin/mdadm -I $env{DEVNAME} --export $devnode --offroot ${DEVLINKS}" for IMPORT (char 58: invalid substitution type), ignoring, but please fix it.
systemd-udevd[217399]: /etc/udev/rules.d/73-special-net-names.rules:14 Invalid value "/bin/sh -ec 'D=${DEVPATH#*/vio/}; D=${D%%%%/*}; D=${D#????}; D=${D#0}; D=${D#0}; D=${D#0}; D=${D#0}; echo ${D:-0}'" for PROGRAM (char 16: invalid substitution type), ignoring, but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/84-nm-drivers.rules:10 Invalid value "/bin/sh -c 'ethtool -i $1 | sed -n s/^driver:\ //p' -- $env{INTERFACE}" for PROGRAM (char 24: invalid substitution type), ignoring, but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/90-libgpod.rules:19 IMPORT key takes '==' or '!=' operator, assuming '==', but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/90-libgpod.rules:23 IMPORT key takes '==' or '!=' operator, assuming '==', but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/99-vmware-scsi-udev.rules:5 Invalid value "/bin/sh -c 'echo 180 >/sys$DEVPATH/device/timeout'" for RUN (char 27: invalid substitution type), ignoring, but please fix it.
systemd-udevd[217399]: /usr/lib/udev/rules.d/99-vmware-scsi-udev.rules:6 Invalid value "/bin/sh -c 'echo 180 >/sys$DEVPATH/device/timeout'" for RUN (char 27: invalid substitution type), ignoring, but please fix it.
systemd[1]: Started udev Kernel Device Manager.

diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
index 0f74fe340a7ef23ed2a5667e4bd769e19f9e60e7..025a37bc26b89213f0b268ed6cf3b235ed5dca69 100644 (file)
--- a/src/udev/udev-event.c
+++ b/src/udev/udev-event.c
@@ -484,29 +484,44 @@ ssize_t udev_event_apply_format(UdevEvent *event,
         return size;
 }
 
-int udev_check_format(const char *s) {
+int udev_check_format(const char *value, size_t *offset, const char **hint) {
         FormatSubstitutionType type;
+        const char *s = value;
         char attr[UTIL_PATH_SIZE];
         int r;
 
         while (*s) {
                 r = get_subst_type(&s, true, &type, attr);
-                if (r < 0)
+                if (r < 0) {
+                        if (offset)
+                                *offset = s - value;
+                        if (hint)
+                                *hint = "invalid substitution type";
                         return r;
-                if (r == 0) {
+                } else if (r == 0) {
                         s++;
                         continue;
                 }
 
-                if (IN_SET(type, FORMAT_SUBST_ATTR, FORMAT_SUBST_ENV) && isempty(attr))
+                if (IN_SET(type, FORMAT_SUBST_ATTR, FORMAT_SUBST_ENV) && isempty(attr)) {
+                        if (offset)
+                                *offset = s - value;
+                        if (hint)
+                                *hint = "attribute value missing";
                         return -EINVAL;
+                }
 
                 if (type == FORMAT_SUBST_RESULT && !isempty(attr)) {
                         unsigned i;
 
                         r = safe_atou_optional_plus(attr, &i);
-                        if (r < 0)
+                        if (r < 0) {
+                                if (offset)
+                                        *offset = s - value;
+                                if (hint)
+                                        *hint = "attribute value not a valid number";
                                 return r;
+                        }
                 }
         }
 

diff --git a/src/udev/udev-event.h b/src/udev/udev-event.h
index eca4ec8a9d021130a7946eaa1380edc03a210bfe..a05e1252595a5fd41893d82d307a3d9e483e64aa 100644 (file)
--- a/src/udev/udev-event.h
+++ b/src/udev/udev-event.h
@@ -51,7 +51,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(UdevEvent*, udev_event_free);
 ssize_t udev_event_apply_format(UdevEvent *event,
                                 const char *src, char *dest, size_t size,
                                 bool replace_whitespace);
-int udev_check_format(const char *s);
+int udev_check_format(const char *value, size_t *offset, const char **hint);
 int udev_event_spawn(UdevEvent *event,
                      usec_t timeout_usec,
                      bool accept_failure,

diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
index 40f743a033c67277c172bb8e2d48bffde82974bf..66208dc2862a79a8cc3f17fdda2a3c18f814bd33 100644 (file)
--- a/src/udev/udev-rules.c
+++ b/src/udev/udev-rules.c
@@ -228,10 +228,10 @@ struct UdevRules {
         log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),           \
                               "Invalid attribute \"%s\" for %s, ignoring, but please fix it.", \
                               attr, key)
-#define log_token_invalid_value(rules, key, value)                      \
+#define log_token_invalid_value(rules, key, value, offset, hint)        \
         log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),           \
-                              "Invalid value \"%s\" for %s, ignoring, but please fix it.", \
-                              value, key)
+                              "Invalid value \"%s\" for %s (char %zu: %s), ignoring, but please fix it.", \
+                              value, key, offset, hint)
 
 static void log_unknown_owner(sd_device *dev, UdevRules *rules, int error, const char *entity, const char *name) {
         if (IN_SET(abs(error), ENOENT, ESRCH))
@@ -510,6 +510,16 @@ static int rule_line_add_token(UdevRuleLine *rule_line, UdevRuleTokenType type,
         return 0;
 }
 
+static void check_value_format_and_warn(UdevRules *rules, const char *key, const char *value, bool nonempty) {
+        size_t offset;
+        const char *hint;
+
+        if (nonempty && isempty(value))
+                log_token_invalid_value(rules, key, value, (size_t) 0, "empty value");
+        else if (udev_check_format(value, &offset, &hint) < 0)
+                log_token_invalid_value(rules, key, value, offset + 1, hint);
+}
+
 static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOperatorType op, char *value) {
         bool is_match = IN_SET(op, OP_MATCH, OP_NOMATCH);
         UdevRuleLine *rule_line;
@@ -551,9 +561,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                         return log_token_invalid_op(rules, key);
 
                 if (!is_match) {
-                        if (udev_check_format(value) < 0)
-                                log_token_invalid_value(rules, key, value);
-
+                        check_value_format_and_warn(rules, key, value, false);
                         r = rule_line_add_token(rule_line, TK_A_DEVLINK, op, value, NULL);
                 } else
                         r = rule_line_add_token(rule_line, TK_M_DEVLINK, op, value, NULL);
@@ -574,8 +582,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                         if (isempty(value))
                                 return log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),
                                                              "Ignoring NAME=\"\", as udev will not delete any device nodes.");
-                        if (udev_check_format(value) < 0)
-                                log_token_invalid_value(rules, key, value);
+                        check_value_format_and_warn(rules, key, value, false);
 
                         r = rule_line_add_token(rule_line, TK_A_NAME, op, value, NULL);
                 } else
@@ -597,8 +604,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                                 return log_token_error_errno(rules, SYNTHETIC_ERRNO(EINVAL),
                                                              "Invalid ENV attribute. '%s' cannot be set.", attr);
 
-                        if (udev_check_format(value) < 0)
-                                log_token_invalid_value(rules, key, value);
+                        check_value_format_and_warn(rules, key, value, false);
+
                         r = rule_line_add_token(rule_line, TK_A_ENV, op, value, attr);
                 } else
                         r = rule_line_add_token(rule_line, TK_M_ENV, op, value, attr);
@@ -611,8 +618,8 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                 }
 
                 if (!is_match) {
-                        if (isempty(value) || udev_check_format(value) < 0)
-                                log_token_invalid_value(rules, key, value);
+                        check_value_format_and_warn(rules, key, value, true);
+
                         r = rule_line_add_token(rule_line, TK_A_TAG, op, value, NULL);
                 } else
                         r = rule_line_add_token(rule_line, TK_M_TAG, op, value, NULL);
@@ -636,7 +643,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
         } else if (streq(key, "ATTR")) {
                 if (isempty(attr))
                         return log_token_invalid_attr(rules, key);
-                if (udev_check_format(attr) < 0)
+                if (udev_check_format(attr, NULL, NULL) < 0)
                         log_token_invalid_attr_format(rules, key, attr);
                 if (op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
@@ -646,16 +653,14 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                 }
 
                 if (!is_match) {
-                        if (udev_check_format(value) < 0)
-                                log_token_invalid_value(rules, key, value);
-
+                        check_value_format_and_warn(rules, key, value, false);
                         r = rule_line_add_token(rule_line, TK_A_ATTR, op, value, attr);
                 } else
                         r = rule_line_add_token(rule_line, TK_M_ATTR, op, value, attr);
         } else if (streq(key, "SYSCTL")) {
                 if (isempty(attr))
                         return log_token_invalid_attr(rules, key);
-                if (udev_check_format(attr) < 0)
+                if (udev_check_format(attr, NULL, NULL) < 0)
                         log_token_invalid_attr_format(rules, key, attr);
                 if (op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
@@ -665,9 +670,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                 }
 
                 if (!is_match) {
-                        if (udev_check_format(value) < 0)
-                                log_token_invalid_value(rules, key, value);
-
+                        check_value_format_and_warn(rules, key, value, false);
                         r = rule_line_add_token(rule_line, TK_A_SYSCTL, op, value, attr);
                 } else
                         r = rule_line_add_token(rule_line, TK_M_SYSCTL, op, value, attr);
@@ -695,7 +698,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
         } else if (streq(key, "ATTRS")) {
                 if (isempty(attr))
                         return log_token_invalid_attr(rules, key);
-                if (udev_check_format(attr) < 0)
+                if (udev_check_format(attr, NULL, NULL) < 0)
                         log_token_invalid_attr_format(rules, key, attr);
                 if (!is_match)
                         return log_token_invalid_op(rules, key);
@@ -721,8 +724,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                         if (r < 0)
                                 return log_token_error_errno(rules, r, "Failed to parse mode '%s': %m", attr);
                 }
-                if (isempty(value) || udev_check_format(value) < 0)
-                        log_token_invalid_value(rules, key, value);
+                check_value_format_and_warn(rules, key, value, true);
                 if (!is_match)
                         return log_token_invalid_op(rules, key);
 
@@ -730,8 +732,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
         } else if (streq(key, "PROGRAM")) {
                 if (attr)
                         return log_token_invalid_attr(rules, key);
-                if (isempty(value) || udev_check_format(value) < 0)
-                        log_token_invalid_value(rules, key, value);
+                check_value_format_and_warn(rules, key, value, true);
                 if (op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
                 if (!is_match) {
@@ -746,8 +747,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
         } else if (streq(key, "IMPORT")) {
                 if (isempty(attr))
                         return log_token_invalid_attr(rules, key);
-                if (isempty(value) || udev_check_format(value) < 0)
-                        log_token_invalid_value(rules, key, value);
+                check_value_format_and_warn(rules, key, value, true);
                 if (op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
                 if (!is_match) {
@@ -849,8 +849,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
 
                         r = rule_line_add_token(rule_line, TK_A_OWNER_ID, op, NULL, UID_TO_PTR(uid));
                 } else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER) {
-                        if (isempty(value) || udev_check_format(value) < 0)
-                                log_token_invalid_value(rules, key, value);
+                        check_value_format_and_warn(rules, key, value, true);
                         r = rule_line_add_token(rule_line, TK_A_OWNER, op, value, NULL);
                 } else {
                         log_token_debug(rules, "Resolving user name is disabled, ignoring %s=%s", key, value);
@@ -878,8 +877,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
 
                         r = rule_line_add_token(rule_line, TK_A_GROUP_ID, op, NULL, GID_TO_PTR(gid));
                 } else if (rules->resolve_name_timing != RESOLVE_NAME_NEVER) {
-                        if (isempty(value) || udev_check_format(value) < 0)
-                                log_token_invalid_value(rules, key, value);
+                        check_value_format_and_warn(rules, key, value, true);
                         r = rule_line_add_token(rule_line, TK_A_GROUP, op, value, NULL);
                 } else {
                         log_token_debug(rules, "Resolving group name is disabled, ignoring %s=%s", key, value);
@@ -900,15 +898,13 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
                 if (parse_mode(value, &mode) >= 0)
                         r = rule_line_add_token(rule_line, TK_A_MODE_ID, op, NULL, MODE_TO_PTR(mode));
                 else {
-                        if (isempty(value) || udev_check_format(value) < 0)
-                                log_token_invalid_value(rules, key, value);
+                        check_value_format_and_warn(rules, key, value, true);
                         r = rule_line_add_token(rule_line, TK_A_MODE, op, value, NULL);
                 }
         } else if (streq(key, "SECLABEL")) {
                 if (isempty(attr))
                         return log_token_invalid_attr(rules, key);
-                if (isempty(value) || udev_check_format(value) < 0)
-                        log_token_invalid_value(rules, key, value);
+                check_value_format_and_warn(rules, key, value, true);
                 if (is_match || op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
                 if (op == OP_ASSIGN_FINAL) {
@@ -920,8 +916,7 @@ static int parse_token(UdevRules *rules, const char *key, char *attr, UdevRuleOp
         } else if (streq(key, "RUN")) {
                 if (is_match || op == OP_REMOVE)
                         return log_token_invalid_op(rules, key);
-                if (isempty(value) || udev_check_format(value) < 0)
-                        log_token_invalid_value(rules, key, value);
+                check_value_format_and_warn(rules, key, value, true);
                 if (!attr || streq(attr, "program"))
                         r = rule_line_add_token(rule_line, TK_A_RUN_PROGRAM, op, value, NULL);
                 else if (streq(attr, "builtin")) {