udev-rules: check token delimiters

When udev_rules_parse_file() is called by udevadm verify, issue warnings
about the following conditions in udev rules:
* the first token in the rule is preceded with a comma
* the last token in the rule is followed by a comma
* there is no comma between tokens
* there is no whitespace between tokens
* there is more than a single comma between tokens
* there is whitespace between a token and a comma
* there is no whitespace after comma

src/udev/udev-rules.c

@@ -1069,6 +1069,51 @@ static UdevRuleOperatorType parse_operator(const char *op) {
         return _OP_TYPE_INVALID;
 }
 
+static void check_token_delimiters(UdevRuleLine *rule_line, const char *line) {
+        assert(rule_line);
+
+        size_t n_comma = 0;
+        bool ws_before_comma = false, ws_after_comma = false;
+        const char *p;
+
+        for (p = line; !isempty(p); ++p) {
+                if (*p == ',')
+                        ++n_comma;
+                else if (strchr(WHITESPACE, *p)) {
+                        if (n_comma > 0)
+                                ws_after_comma = true;
+                        else
+                                ws_before_comma = true;
+                } else
+                        break;
+        }
+
+        if (line == rule_line->line) {
+                /* this is the first token of the rule */
+                if (n_comma > 0)
+                        log_line_warning(rule_line, "Stray leading comma.");
+        } else if (isempty(p)) {
+                /* there are no more tokens in the rule */
+                if (n_comma > 0)
+                        log_line_warning(rule_line, "Stray trailing comma.");
+        } else {
+                /* single comma is expected */
+                if (n_comma == 0)
+                        log_line_warning(rule_line, "A comma between tokens is expected.");
+                else if (n_comma > 1)
+                        log_line_warning(rule_line, "More than one comma between tokens.");
+
+                /* whitespace after comma is expected */
+                if (n_comma > 0) {
+                        if (ws_before_comma)
+                                log_line_warning(rule_line, "Stray whitespace before comma.");
+                        if (!ws_after_comma)
+                                log_line_warning(rule_line, "Whitespace after comma is expected.");
+                } else if (!ws_before_comma && !ws_after_comma)
+                        log_line_warning(rule_line, "Whitespace between tokens is expected.");
+        }
+}
+
 static int parse_line(char **line, char **ret_key, char **ret_attr, UdevRuleOperatorType *ret_op, char **ret_value) {
         char *key_begin, *key_end, *attr, *tmp;
         UdevRuleOperatorType op;
@@ -1140,7 +1185,7 @@ static void sort_tokens(UdevRuleLine *rule_line) {
         }
 }
 
-static int rule_add_line(UdevRuleFile *rule_file, const char *line_str, unsigned line_nr) {
+static int rule_add_line(UdevRuleFile *rule_file, const char *line_str, unsigned line_nr, bool extra_checks) {
         _cleanup_(udev_rule_line_freep) UdevRuleLine *rule_line = NULL;
         _cleanup_free_ char *line = NULL;
         char *p;
@@ -1172,6 +1217,9 @@ static int rule_add_line(UdevRuleFile *rule_file, const char *line_str, unsigned
                 char *key, *attr, *value;
                 UdevRuleOperatorType op;
 
+                if (extra_checks)
+                        check_token_delimiters(rule_line, p);
+
                 r = parse_line(&p, &key, &attr, &op, &value);
                 if (r < 0)
                         return log_line_error_errno(rule_line, r, "Invalid key/value pair, ignoring.");
@@ -1457,7 +1505,7 @@ int udev_rules_parse_file(UdevRules *rules, const char *filename, bool extra_che
                 if (ignore_line)
                         log_file_error(rule_file, line_nr, "Line is too long, ignored");
                 else if (len > 0)
-                        (void) rule_add_line(rule_file, line, line_nr);
+                        (void) rule_add_line(rule_file, line, line_nr, extra_checks);
 
                 continuation = mfree(continuation);
                 ignore_line = false;

test/units/testsuite-17.11.sh

@@ -287,6 +287,15 @@ test_syntax_error 'KERNEL=="|a|b", KERNEL=="b|a|", NAME="c"' 'duplicate expressi
 # shellcheck disable=SC2016
 test_syntax_error 'ENV{DISKSEQ}=="?*", ENV{DEVTYPE}!="partition", ENV{DISKSEQ}=="?*", ENV{ID_IGNORE_DISKSEQ}!="1", SYMLINK+="disk/by-diskseq/$env{DISKSEQ}"' \
                   'duplicate expressions'
+test_syntax_error ',ACTION=="a", NAME="b"' 'Stray leading comma.'
+test_syntax_error ' ,ACTION=="a", NAME="b"' 'Stray leading comma.'
+test_syntax_error ', ACTION=="a", NAME="b"' 'Stray leading comma.'
+test_syntax_error 'ACTION=="a", NAME="b",' 'Stray trailing comma.'
+test_syntax_error 'ACTION=="a", NAME="b", ' 'Stray trailing comma.'
+test_syntax_error 'ACTION=="a" NAME="b"' 'A comma between tokens is expected.'
+test_syntax_error 'ACTION=="a",, NAME="b"' 'More than one comma between tokens.'
+test_syntax_error 'ACTION=="a" , NAME="b"' 'Stray whitespace before comma.'
+test_syntax_error 'ACTION=="a",NAME="b"' 'Whitespace after comma is expected.'
 
 cat >"${rules}" <<'EOF'
 KERNEL=="a|b", KERNEL=="a|c", NAME="d"
@@ -348,6 +357,28 @@ EOF
 cp "${workdir}/default_output_1_fail" "${exo}"
 assert_1 "${rules}"
 
+cat >"${rules}" <<'EOF'
+ACTION=="a"NAME="b"
+EOF
+cat >"${exp}" <<EOF
+${rules}:1 A comma between tokens is expected.
+${rules}:1 Whitespace between tokens is expected.
+${rules}: udev rules check failed
+EOF
+cp "${workdir}/default_output_1_fail" "${exo}"
+assert_1 "${rules}"
+
+cat >"${rules}" <<'EOF'
+ACTION=="a" ,NAME="b"
+EOF
+cat >"${exp}" <<EOF
+${rules}:1 Stray whitespace before comma.
+${rules}:1 Whitespace after comma is expected.
+${rules}: udev rules check failed
+EOF
+cp "${workdir}/default_output_1_fail" "${exo}"
+assert_1 "${rules}"
+
 # udevadm verify --root
 sed "s|sample-[0-9]*.rules|${workdir}/${rules_dir}/&|" sample-*.exp >"${workdir}/${exp}"
 cd -