From 1bf3dd41539902d0644d6a6d05ee8653787f0690 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 1 Jun 2023 10:10:00 +0900 Subject: [PATCH] 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 a1c97734130..900bdac4b32 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 2e2fe5a58e8..0606160c66b 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 96fb59af891..37e9f28a584 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 646d4829c11..6c1ac488e98 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 cf9ad67b048..32202508f31 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 1ef6fa528fd..42b925f60b3 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}" -- 2.47.3