shaba/systemd
1
0
Fork 0
mirror of https://github.com/systemd/systemd.git synced 2024-12-23 21:35:11 +03:00
Code

condition: change operator logic to use $= instead of =$ for glob comparisons

Browse Source 
So this is a bit of a bikeshedding thing. But I think we should do this
nonetheless, before this is released.

Playing around with the glob matches I realized that "=$" is really hard
to grep for, since in shell code it's an often seen construct. Also,
when reading code I often found myself thinking first that the "$"
belongs to the rvalue instead of the operator, in a variable expansion
scheme.

If we move the $ character to the left hand, I think we are on the safer
side, since usually lvalues are much more restricted in character sets
than rvalues (at least most programming languages do enforce limits on
the character set for identifiers).

It makes it much easier to grep for the new operator, and easier to read
too. Example:

before:
    ConditionOSRelease=ID=$fedora-*
after:
    ConditionOSRelease=ID$=fedora-*
This commit is contained in:
Lennart Poettering 2022-08-29 13:42:44 +02:00
parent 06219747f5
commit 71a3ff036b
3 changed files with 14 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
man/systemd.unit.xml
View File

@ -1251,9 +1251,9 @@
<literal>operator</literal> is one of <literal>&lt;</literal>, <literal>&lt;=</literal>,
<literal>&gt;=</literal>, <literal>&gt;</literal>, <literal>==</literal>,
<literal>&lt;&gt;</literal> for version comparison, <literal>=</literal> and <literal>!=</literal>
for literal string comparison, or <literal>=$</literal>, <literal>!=$</literal> for shell-style
for literal string comparison, or <literal>$=</literal>, <literal>!$=</literal> for shell-style
glob comparison. <literal>value</literal> is the expected value of the SMBIOS field value
(possibly containing shell style globs in case <literal>=$</literal>/<literal>!=$</literal> is
(possibly containing shell style globs in case <literal>$=</literal>/<literal>!$=</literal> is
used).</para>
</listitem>
</varlistentry>
@ -1337,8 +1337,8 @@
expressions. Each expression starts with one of <literal>=</literal> or <literal>!=</literal> for
string comparisons, <literal>&lt;</literal>, <literal>&lt;=</literal>, <literal>==</literal>,
<literal>&lt;&gt;</literal>, <literal>&gt;=</literal>, <literal>&gt;</literal> for a relative
version comparison, or <literal>=$</literal>, <literal>!=$</literal> for a shell-style glob
match. If no operator is specified <literal>=$</literal> is implied.</para>
version comparison, or <literal>$=</literal>, <literal>!$=</literal> for a shell-style glob
match. If no operator is specified <literal>$=</literal> is implied.</para>
<para>Note that using the kernel version string is an unreliable way to determine which features
are supported by a kernel, because of the widespread practice of backporting drivers, features, and
@ -1704,7 +1704,7 @@
with <literal>&lt;</literal>, <literal>&lt;=</literal>, <literal>==</literal>,
<literal>&lt;&gt;</literal>, <literal>&gt;=</literal>, <literal>&gt;</literal>), and shell-style
wildcard comparisons (<literal>*</literal>, <literal>?</literal>, <literal>[]</literal>) are
supported with the <literal>=$</literal> (match) and <literal>!=$</literal> (non-match).</para>
supported with the <literal>$=</literal> (match) and <literal>!$=</literal> (non-match).</para>
</listitem>
</varlistentry>

4
src/shared/compare-operator.c
View File

@ -12,8 +12,8 @@ CompareOperator parse_compare_operator(const char **s, CompareOperatorParseFlags
CompareOperatorParseFlags valid_mask; /* If this operator appears when flags in mask not set, fail */
CompareOperatorParseFlags need_mask; /* Skip over this operattor when flags in mask not set */
} table[] = {
{ COMPARE_FNMATCH_EQUAL, "=$", .valid_mask = COMPARE_ALLOW_FNMATCH },
{ COMPARE_FNMATCH_UNEQUAL, "!=$", .valid_mask = COMPARE_ALLOW_FNMATCH },
{ COMPARE_FNMATCH_EQUAL, "$=", .valid_mask = COMPARE_ALLOW_FNMATCH },
{ COMPARE_FNMATCH_UNEQUAL, "!$=", .valid_mask = COMPARE_ALLOW_FNMATCH },
{ COMPARE_UNEQUAL, "<>" },
{ COMPARE_LOWER_OR_EQUAL, "<=" },

14
src/test/test-condition.c
View File

@ -359,31 +359,31 @@ TEST(condition_test_firmware_smbios_field) {
const char *quote = strchr(bios_vendor, ' ') ? "\"" : "";
/* Test equality / inequality using fnmatch() */
expression = strjoina("smbios-field(bios_vendor =$ ", quote, bios_vendor, quote, ")");
expression = strjoina("smbios-field(bios_vendor $= ", quote, bios_vendor, quote, ")");
condition = condition_new(CONDITION_FIRMWARE, expression, false, false);
assert_se(condition);
assert_se(condition_test(condition, environ) > 0);
condition_free(condition);
expression = strjoina("smbios-field(bios_vendor=$", quote, bios_vendor, quote, ")");
expression = strjoina("smbios-field(bios_vendor$=", quote, bios_vendor, quote, ")");
condition = condition_new(CONDITION_FIRMWARE, expression, false, false);
assert_se(condition);
assert_se(condition_test(condition, environ) > 0);
condition_free(condition);
expression = strjoina("smbios-field(bios_vendor !=$ ", quote, bios_vendor, quote, ")");
expression = strjoina("smbios-field(bios_vendor !$= ", quote, bios_vendor, quote, ")");
condition = condition_new(CONDITION_FIRMWARE, expression, false, false);
assert_se(condition);
assert_se(condition_test(condition, environ) == 0);
condition_free(condition);
expression = strjoina("smbios-field(bios_vendor!=$", quote, bios_vendor, quote, ")");
expression = strjoina("smbios-field(bios_vendor!$=", quote, bios_vendor, quote, ")");
condition = condition_new(CONDITION_FIRMWARE, expression, false, false);
assert_se(condition);
assert_se(condition_test(condition, environ) == 0);
condition_free(condition);
expression = strjoina("smbios-field(bios_vendor =$ ", quote, bios_vendor, "*", quote, ")");
expression = strjoina("smbios-field(bios_vendor $= ", quote, bios_vendor, "*", quote, ")");
condition = condition_new(CONDITION_FIRMWARE, expression, false, false);
assert_se(condition);
assert_se(condition_test(condition, environ) > 0);
@ -1174,13 +1174,13 @@ TEST(condition_test_os_release) {
condition_free(condition);
/* Test fnmatch() operators */
key_value_pair = strjoina(os_release_pairs[0], "=$", quote, os_release_pairs[1], quote);
key_value_pair = strjoina(os_release_pairs[0], "$=", quote, os_release_pairs[1], quote);
condition = condition_new(CONDITION_OS_RELEASE, key_value_pair, false, false);
assert_se(condition);
assert_se(condition_test(condition, environ) > 0);
condition_free(condition);
key_value_pair = strjoina(os_release_pairs[0], "!=$", quote, os_release_pairs[1], quote);
key_value_pair = strjoina(os_release_pairs[0], "!$=", quote, os_release_pairs[1], quote);
condition = condition_new(CONDITION_OS_RELEASE, key_value_pair, false, false);
assert_se(condition);
assert_se(condition_test(condition, environ) == 0);