From 4134614fc0c0dd172b8aa626c7f5c75489ca9554 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 1 Jun 2023 10:06:59 +0900 Subject: [PATCH 1/3] udev-rules: terminate log messages with period --- src/udev/udev-rules.c | 22 ++++++------- src/udev/udevadm-verify.c | 4 +-- test/units/testsuite-17.11.sh | 60 +++++++++++++++++------------------ 3 files changed, 43 insertions(+), 43 deletions(-) diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index e5a25bd119..646d4829c1 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -304,7 +304,7 @@ static void log_unknown_owner(sd_device *dev, UdevRuleLine *line, int error, con if (IN_SET(abs(error), ENOENT, ESRCH)) log_udev_rule_internal(dev, line->rule_file, line->line_number, LOG_ERR, error, - "Unknown %s '%s', ignoring", entity, name); + "Unknown %s '%s', ignoring.", entity, name); else log_udev_rule_internal(dev, line->rule_file, line->line_number, LOG_ERR, error, "Failed to resolve %s '%s', ignoring: %m", entity, name); @@ -844,7 +844,7 @@ static int parse_token(UdevRuleLine *rule_line, const char *key, char *attr, Ude cmd = udev_builtin_lookup(value); if (cmd >= 0) { - log_line_debug(rule_line, "Found builtin command '%s' for %s, replacing attribute", value, key); + log_line_debug(rule_line, "Found builtin command '%s' for %s, replacing attribute.", value, key); r = rule_line_add_token(rule_line, TK_M_IMPORT_BUILTIN, op, value, UDEV_BUILTIN_CMD_TO_PTR(cmd)); } else r = rule_line_add_token(rule_line, TK_M_IMPORT_PROGRAM, op, value, NULL); @@ -940,7 +940,7 @@ static int parse_token(UdevRuleLine *rule_line, const char *key, char *attr, Ude check_value_format_and_warn(rule_line, key, value, true); r = rule_line_add_token(rule_line, TK_A_OWNER, op, value, NULL); } else { - log_line_debug(rule_line, "User name resolution is disabled, ignoring %s=%s", key, value); + log_line_debug(rule_line, "User name resolution is disabled, ignoring %s=\"%s\".", key, value); return 0; } } else if (streq(key, "GROUP")) { @@ -968,7 +968,7 @@ static int parse_token(UdevRuleLine *rule_line, const char *key, char *attr, Ude check_value_format_and_warn(rule_line, key, value, true); r = rule_line_add_token(rule_line, TK_A_GROUP, op, value, NULL); } else { - log_line_debug(rule_line, "Resolving group name is disabled, ignoring GROUP=\"%s\"", value); + log_line_debug(rule_line, "Resolving group name is disabled, ignoring GROUP=\"%s\".", value); return 0; } } else if (streq(key, "MODE")) { @@ -1013,7 +1013,7 @@ static int parse_token(UdevRuleLine *rule_line, const char *key, char *attr, Ude cmd = udev_builtin_lookup(value); if (cmd < 0) return log_line_error_errno(rule_line, SYNTHETIC_ERRNO(EINVAL), - "Unknown builtin command '%s', ignoring", value); + "Unknown builtin command '%s', ignoring.", value); r = rule_line_add_token(rule_line, TK_A_RUN_BUILTIN, op, value, UDEV_BUILTIN_CMD_TO_PTR(cmd)); } else return log_line_invalid_attr(rule_line, key); @@ -1043,7 +1043,7 @@ static int parse_token(UdevRuleLine *rule_line, const char *key, char *attr, Ude SET_FLAG(rule_line->type, LINE_HAS_LABEL, true); return 1; } else - return log_line_error_errno(rule_line, SYNTHETIC_ERRNO(EINVAL), "Invalid key '%s'", key); + return log_line_error_errno(rule_line, SYNTHETIC_ERRNO(EINVAL), "Invalid key '%s'.", key); if (r < 0) return log_oom(); @@ -1274,7 +1274,7 @@ static void rule_resolve_goto(UdevRuleFile *rule_file) { } if (!line->goto_line) { - log_line_error(line, "GOTO=\"%s\" has no matching label, ignoring", + log_line_error(line, "GOTO=\"%s\" has no matching label, ignoring.", line->goto_label); SET_FLAG(line->type, LINE_HAS_GOTO, false); @@ -1424,11 +1424,11 @@ static void udev_check_conflicts_duplicates(UdevRuleLine *line) { if (new_duplicates) { duplicates = new_duplicates; - log_line_warning(line, "duplicate expressions"); + log_line_warning(line, "duplicate expressions."); } if (new_conflicts) { conflicts = new_conflicts; - log_line_error(line, "conflicting match expressions, the line has no effect"); + log_line_error(line, "conflicting match expressions, the line has no effect."); } if (conflicts && duplicates) return; @@ -1544,7 +1544,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"); + 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, extra_checks); @@ -1554,7 +1554,7 @@ int udev_rules_parse_file(UdevRules *rules, const char *filename, bool extra_che if (continuation) log_file_error(rule_file, line_nr, - "Unexpected EOF after line continuation, line ignored"); + "Unexpected EOF after line continuation, line ignored."); rule_resolve_goto(rule_file); diff --git a/src/udev/udevadm-verify.c b/src/udev/udevadm-verify.c index 58d35fef13..cf9ad67b04 100644 --- a/src/udev/udevadm-verify.c +++ b/src/udev/udevadm-verify.c @@ -122,7 +122,7 @@ static int verify_rules_file(UdevRules *rules, const char *fname) { unsigned mask = (1U << LOG_ERR) | (1U << LOG_WARNING); if (issues & mask) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "%s: udev rules check failed", fname); + "%s: udev rules check failed.", fname); return 0; } @@ -215,7 +215,7 @@ int verify_main(int argc, char *argv[], void *userdata) { return log_error_errno(r, "Failed to enumerate rules files: %m"); if (arg_root && strv_isempty(files)) return log_error_errno(SYNTHETIC_ERRNO(ENOENT), - "No rules files found in %s", arg_root); + "No rules files found in %s.", arg_root); return verify_rules(rules, files); } diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index 56e9ac24ac..765a30df29 100755 --- a/test/units/testsuite-17.11.sh +++ b/test/units/testsuite-17.11.sh @@ -168,16 +168,16 @@ assert_0 "${rules}" printf 'RUN+="/bin/true"%8176s\\\n #\n' ' ' ' ' >"${rules}" echo >>"${rules}" cat >"${exp}" <"${rules}" cat >"${exp}" <"${rules}" cat >"${exp}" <"${rules}" cat >"${exp}" <"${exp}" <"${exp}" <"${rules}" <<'EOF' KERNEL!="", KERNEL=="?*", KERNEL=="", NAME="a" EOF cat >"${exp}" <"${exp}" <"${exp}" < Date: Thu, 1 Jun 2023 11:48:09 +0900 Subject: [PATCH 2/3] test: drop unnecessary copy of expected output --- test/units/testsuite-17.11.sh | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index 765a30df29..1ef6fa528f 100755 --- a/test/units/testsuite-17.11.sh +++ b/test/units/testsuite-17.11.sh @@ -81,6 +81,8 @@ assert_1() { if [ -f "${exo}" ]; then diff -u "${exo}" "${out}" + elif [ -f "${rules}" ]; then + diff -u "${workdir}/default_output_1_fail" "${out}" fi next_test_number @@ -127,7 +129,6 @@ assert_0 "${rules_dir}" # Directory with a loop. ln -s . "${rules_dir}/loop.rules" -cp "${workdir}/default_output_1_fail" "${exo}" assert_1 "${rules_dir}" rm "${rules_dir}/loop.rules" @@ -155,7 +156,6 @@ assert_0 "${rules}" # Failed to parse rules file ${rules}: No buffer space available printf '%16384s\n' ' ' >"${rules}" echo "Failed to parse rules file ${rules}: No buffer space available" >"${exp}" -cp "${workdir}/default_output_1_fail" "${exo}" assert_1 "${rules}" { @@ -171,7 +171,6 @@ cat >"${exp}" <"${rules}" @@ -179,7 +178,6 @@ cat >"${exp}" <"${rules}" <<'EOF' @@ -364,7 +360,6 @@ cat >"${exp}" <"${rules}" <<'EOF' @@ -378,7 +373,6 @@ ${rules}:1 The line has no effect any more, dropping. ${rules}:2 LABEL="b" is unused. ${rules}: udev rules check failed. EOF -cp "${workdir}/default_output_1_fail" "${exo}" assert_1 "${rules}" cat >"${rules}" <<'EOF' @@ -389,7 +383,6 @@ ${rules}:1 duplicate expressions. ${rules}:1 conflicting match expressions, the line has no effect. ${rules}: udev rules check failed. EOF -cp "${workdir}/default_output_1_fail" "${exo}" assert_1 "${rules}" cat >"${rules}" <<'EOF' @@ -400,7 +393,6 @@ ${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' @@ -411,7 +403,6 @@ ${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 From 1bf3dd41539902d0644d6a6d05ee8653787f0690 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 1 Jun 2023 10:10:00 +0900 Subject: [PATCH 3/3] udev: downgrade log level about style issues And add --no-style switch that make style issues not critical. --- man/udevadm.xml | 8 ++++ shell-completion/bash/udevadm | 2 +- shell-completion/zsh/_udevadm | 1 + src/udev/udev-rules.c | 18 ++++---- src/udev/udevadm-verify.c | 12 ++++++ test/units/testsuite-17.11.sh | 77 +++++++++++++++++++++++------------ 6 files changed, 83 insertions(+), 35 deletions(-) diff --git a/man/udevadm.xml b/man/udevadm.xml index a1c9773413..900bdac4b3 100644 --- a/man/udevadm.xml +++ b/man/udevadm.xml @@ -788,6 +788,14 @@ + + + + Ignore style issues. When specified, even if style issues are found in udev rules files, + the exit status is 0 if no syntactic or semantic errors are found. + + + diff --git a/shell-completion/bash/udevadm b/shell-completion/bash/udevadm index 2e2fe5a58e..0606160c66 100644 --- a/shell-completion/bash/udevadm +++ b/shell-completion/bash/udevadm @@ -64,7 +64,7 @@ _udevadm() { [MONITOR_ARG]='-s --subsystem-match -t --tag-match' [TEST]='-a --action -N --resolve-names' [TEST_BUILTIN]='-a --action' - [VERIFY]='-N --resolve-names --root --no-summary' + [VERIFY]='-N --resolve-names --root --no-summary --no-style' [WAIT]='-t --timeout --initialized=no --removed --settle' [LOCK]='-t --timeout -d --device -b --backing -p --print' ) diff --git a/shell-completion/zsh/_udevadm b/shell-completion/zsh/_udevadm index 96fb59af89..37e9f28a58 100644 --- a/shell-completion/zsh/_udevadm +++ b/shell-completion/zsh/_udevadm @@ -110,6 +110,7 @@ _udevadm_verify(){ {-N+,--resolve-names=}'[When to resolve names.]:resolve:(early never)' \ '--root=[Operate on catalog hierarchy under specified directory]:directories:_directories' \ {--no-summary}'[Do not show summary.]' \ + {--no-style}'[Ignore style issues.]' \ {-h,--help}'[Print help text.]' \ '*::files:_files' } diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c index 646d4829c1..6c1ac488e9 100644 --- a/src/udev/udev-rules.c +++ b/src/udev/udev-rules.c @@ -1091,26 +1091,26 @@ static void check_token_delimiters(UdevRuleLine *rule_line, const char *line) { 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."); + log_line_notice(rule_line, "style: 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."); + log_line_notice(rule_line, "style: stray trailing comma."); } else { /* single comma is expected */ if (n_comma == 0) - log_line_warning(rule_line, "A comma between tokens is expected."); + log_line_notice(rule_line, "style: a comma between tokens is expected."); else if (n_comma > 1) - log_line_warning(rule_line, "More than one comma between tokens."); + log_line_notice(rule_line, "style: 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."); + log_line_notice(rule_line, "style: stray whitespace before comma."); if (!ws_after_comma) - log_line_warning(rule_line, "Whitespace after comma is expected."); + log_line_notice(rule_line, "style: whitespace after comma is expected."); } else if (!ws_before_comma && !ws_after_comma) - log_line_warning(rule_line, "Whitespace between tokens is expected."); + log_line_notice(rule_line, "style: whitespace between tokens is expected."); } } @@ -1281,7 +1281,7 @@ static void rule_resolve_goto(UdevRuleFile *rule_file) { line->goto_label = NULL; if ((line->type & ~(LINE_HAS_LABEL|LINE_IS_REFERENCED)) == 0) { - log_line_notice(line, "The line has no effect any more, dropping."); + log_line_warning(line, "The line has no effect any more, dropping."); /* LINE_IS_REFERENCED implies LINE_HAS_LABEL */ if (line->type & LINE_HAS_LABEL) udev_rule_line_clear_tokens(line); @@ -1400,7 +1400,7 @@ static void udev_check_unused_labels(UdevRuleLine *line) { if (FLAGS_SET(line->type, LINE_HAS_LABEL) && !FLAGS_SET(line->type, LINE_IS_REFERENCED)) - log_line_warning(line, "LABEL=\"%s\" is unused.", line->label); + log_line_notice(line, "style: LABEL=\"%s\" is unused.", line->label); } static void udev_check_conflicts_duplicates(UdevRuleLine *line) { diff --git a/src/udev/udevadm-verify.c b/src/udev/udevadm-verify.c index cf9ad67b04..32202508f3 100644 --- a/src/udev/udevadm-verify.c +++ b/src/udev/udevadm-verify.c @@ -21,6 +21,7 @@ static ResolveNameTiming arg_resolve_name_timing = RESOLVE_NAME_EARLY; static char *arg_root = NULL; static bool arg_summary = true; +static bool arg_style = true; STATIC_DESTRUCTOR_REGISTER(arg_root, freep); @@ -39,6 +40,7 @@ static int help(void) { " -N --resolve-names=early|never When to resolve names\n" " --root=PATH Operate on an alternate filesystem root\n" " --no-summary Do not show summary\n" + " --no-style Ignore style issues\n" "\nSee the %s for details.\n", program_invocation_short_name, ansi_highlight(), @@ -52,6 +54,7 @@ static int parse_argv(int argc, char *argv[]) { enum { ARG_ROOT = 0x100, ARG_NO_SUMMARY, + ARG_NO_STYLE, }; static const struct option options[] = { { "help", no_argument, NULL, 'h' }, @@ -59,6 +62,7 @@ static int parse_argv(int argc, char *argv[]) { { "resolve-names", required_argument, NULL, 'N' }, { "root", required_argument, NULL, ARG_ROOT }, { "no-summary", no_argument, NULL, ARG_NO_SUMMARY }, + { "no-style", no_argument, NULL, ARG_NO_STYLE }, {} }; @@ -95,6 +99,10 @@ static int parse_argv(int argc, char *argv[]) { arg_summary = false; break; + case ARG_NO_STYLE: + arg_style = false; + break; + case '?': return -EINVAL; default: @@ -124,6 +132,10 @@ static int verify_rules_file(UdevRules *rules, const char *fname) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "%s: udev rules check failed.", fname); + if (arg_style && (issues & (1U << LOG_NOTICE))) + return log_warning_errno(SYNTHETIC_ERRNO(EINVAL), + "%s: udev rules have style issues.", fname); + return 0; } diff --git a/test/units/testsuite-17.11.sh b/test/units/testsuite-17.11.sh index 1ef6fa528f..42b925f60b 100755 --- a/test/units/testsuite-17.11.sh +++ b/test/units/testsuite-17.11.sh @@ -56,18 +56,21 @@ next_test_number() { out="sample-${num_str}.out" } -assert_0() { +assert_0_impl() { udevadm verify "$@" >"${out}" if [ -f "${exo}" ]; then diff -u "${exo}" "${out}" elif [ -f "${rules}" ]; then diff -u "${workdir}/default_output_1_success" "${out}" fi +} +assert_0() { + assert_0_impl "$@" next_test_number } -assert_1() { +assert_1_impl() { local rc set +e udevadm verify "$@" >"${out}" 2>"${err}" @@ -84,7 +87,10 @@ assert_1() { elif [ -f "${rules}" ]; then diff -u "${workdir}/default_output_1_fail" "${out}" fi +} +assert_1() { + assert_1_impl "$@" next_test_number } @@ -194,6 +200,22 @@ EOF assert_1 "${rules}" } +test_style_error() { + local rule msg + + rule="$1"; shift + msg="$1"; shift + + printf '%s\n' "${rule}" >"${rules}" + cat >"${exp}" <"${rules}" <<'EOF' KERNEL=="a|b", KERNEL=="a|c", NAME="d" @@ -357,10 +379,11 @@ LABEL="b" LABEL="b" EOF cat >"${exp}" <"${rules}" <<'EOF' GOTO="a" @@ -370,7 +393,7 @@ cat >"${exp}" <"${rules}" <<'EOF' ACTION=="a"NAME="b" EOF cat >"${exp}" <"${rules}" <<'EOF' ACTION=="a" ,NAME="b" EOF cat >"${exp}" <"${workdir}/${exp}"