hwdb: improve and test syntax error messages

Since syntax error are non-fatal, downgrade them to warnings.
Use log_syntax to have uniform formatting including the line number.
State machine states like DATA and MATCH are internal, user-facing
messages should use the names from hwdb(7): match, property, record.
Also change "key/value" to "key-value", since there's no alternative
here, both parts must be present.

[/tmp/tmp.KFwEhm74n4/etc/udev/hwdb.d/10-bad.hwdb:2] Property expected, ignoring record with no properties
[/tmp/tmp.KFwEhm74n4/etc/udev/hwdb.d/10-bad.hwdb:5] Property expected, ignoring record with no properties
[/tmp/tmp.KFwEhm74n4/etc/udev/hwdb.d/10-bad.hwdb:9] Property expected, ignoring record with no properties
[/tmp/tmp.KFwEhm74n4/etc/udev/hwdb.d/10-bad.hwdb:11] Key/value pair expected but got " NO_VALUE", ignoring
[/tmp/tmp.KFwEhm74n4/etc/udev/hwdb.d/10-bad.hwdb:18] Property or empty line expected, got "BAD:7:match at wrong place", ignoring record
[/tmp/tmp.KFwEhm74n4/etc/udev/hwdb.d/10-bad.hwdb:22] Property or empty line expected, got "BAD:8:match at wrong place", ignoring record
[/tmp/tmp.KFwEhm74n4/etc/udev/hwdb.d/10-bad.hwdb:23] Match expected but got indented property " Z=z", ignoring line

squash! hwdb: improve syntax error messages

diff --git a/Makefile.am b/Makefile.am
index 124e1867cde2c568869c3f76e052335c13ae6c1d..cad4f050af17c4564fd1c07f5076205e4bbb962f 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -1700,7 +1700,8 @@ EXTRA_DIST += \
        test/bus-policy/signals.conf \
        test/bus-policy/check-own-rules.conf \
        test/bus-policy/many-rules.conf \
-       test/bus-policy/test.conf
+       test/bus-policy/test.conf \
+       test/hwdb/10-bad.hwdb
 
 
 EXTRA_DIST += \

diff --git a/src/hwdb/hwdb.c b/src/hwdb/hwdb.c
index 0fbde28d105d9807ed3cb95a9f3fcf8fcf6c7a01..6d4a03bedba7dbe172b03f3893f3435533d127fb 100644 (file)
--- a/src/hwdb/hwdb.c
+++ b/src/hwdb/hwdb.c
@@ -39,7 +39,7 @@
 #include "verbs.h"
 
 /*
- * Generic udev properties, key/value database based on modalias strings.
+ * Generic udev properties, key-value database based on modalias strings.
  * Uses a Patricia/radix trie to index all matches for efficient lookup.
  */
 
@@ -70,7 +70,7 @@ struct trie_node {
         struct trie_child_entry *children;
         uint8_t children_count;
 
-        /* sorted array of key/value pairs */
+        /* sorted array of key-value pairs */
         struct trie_value_entry *values;
         size_t values_count;
 };
@@ -81,7 +81,7 @@ struct trie_child_entry {
         struct trie_node *child;
 };
 
-/* value array item with key/value pairs */
+/* value array item with key-value pairs */
 struct trie_value_entry {
         size_t key_off;
         size_t value_off;
@@ -457,10 +457,9 @@ static int insert_data(struct trie *trie, char **match_list, char *line,
         char *value, **entry;
 
         value = strchr(line, '=');
-        if (!value) {
-                log_error("Error, key/value pair expected but got '%s' in '%s':", line, filename);
-                return -EINVAL;
-        }
+        if (!value)
+                return log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                                  "Key-value pair expected but got \"%s\", ignoring", line);
 
         value[0] = '\0';
         value++;
@@ -469,10 +468,9 @@ static int insert_data(struct trie *trie, char **match_list, char *line,
         while (isblank(line[0]) && isblank(line[1]))
                 line++;
 
-        if (line[0] == '\0' || value[0] == '\0') {
-                log_error("Error, empty key or value '%s' in '%s':", line, filename);
-                return -EINVAL;
-        }
+        if (line[0] == '\0' || value[0] == '\0')
+                return log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                                  "Empty %s in \"%s\", ignoring", line[0] == '\0' ? "key" : "value", line);
 
         STRV_FOREACH(entry, match_list)
                 trie_insert(trie, trie->root, *entry, line, value, filename, file_priority, line_number);
@@ -524,7 +522,8 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr
                                 break;
 
                         if (line[0] == ' ') {
-                                log_error("Error, MATCH expected but got '%s' in '%s':", line, filename);
+                                log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                                           "Match expected but got indented property \"%s\", ignoring line", line);
                                 break;
                         }
 
@@ -543,7 +542,9 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr
 
                 case HW_MATCH:
                         if (len == 0) {
-                                log_error("Error, DATA expected but got empty line in '%s':", filename);
+                                log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                                           "Property expected, ignoring record with no properties");
+
                                 state = HW_NONE;
                                 strv_clear(match_list);
                                 break;
@@ -576,7 +577,8 @@ static int import_file(struct trie *trie, const char *filename, uint16_t file_pr
                         }
 
                         if (line[0] != ' ') {
-                                log_error("Error, DATA expected but got '%s' in '%s':", line, filename);
+                                log_syntax(NULL, LOG_WARNING, filename, line_number, EINVAL,
+                                           "Property or empty line expected, got \"%s\", ignoring record", line);
                                 state = HW_NONE;
                                 strv_clear(match_list);
                                 break;

diff --git a/test/hwdb-test.sh b/test/hwdb-test.sh
index bfdafce998441a94e8071eca396b9ad1ce1ff76d..5373930df44430c7e374439e4c9bac57082f6e24 100755 (executable)
--- a/test/hwdb-test.sh
+++ b/test/hwdb-test.sh
@@ -33,6 +33,7 @@ trap "rm -rf '$D'" EXIT INT QUIT PIPE
 mkdir -p "$D/etc/udev"
 ln -s "$ROOTDIR/hwdb" "$D/etc/udev/hwdb.d"
 
+# Test "good" properties" — no warnings or errors allowed
 err=$("$SYSTEMD_HWDB" update --root "$D" 2>&1 >/dev/null) && rc= || rc=$?
 if [ -n "$err" ]; then
     echo "$err"
@@ -47,3 +48,25 @@ if [ ! -e "$D/etc/udev/hwdb.bin" ]; then
     echo "$D/etc/udev/hwdb.bin was not generated"
     exit 1
 fi
+
+# Test "bad" properties" — warnings required, errors not allowed
+rm -f "$D/etc/udev/hwdb.bin" "$D/etc/udev/hwdb.d"
+
+ln -s "$ROOTDIR/test/hwdb" "$D/etc/udev/hwdb.d"
+err=$("$SYSTEMD_HWDB" update --root "$D" 2>&1 >/dev/null) && rc= || rc=$?
+if [ -n "$rc" ]; then
+    echo "$SYSTEMD_HWDB returned $rc"
+    exit $rc
+fi
+if [ -n "$err" ]; then
+    echo "Expected warnings"
+    echo "$err"
+else
+    echo "$SYSTEMD_HWDB unexpectedly printed no warnings"
+    exit 1
+fi
+
+if [ ! -e "$D/etc/udev/hwdb.bin" ]; then
+    echo "$D/etc/udev/hwdb.bin was not generated"
+    exit 1
+fi

diff --git a/test/hwdb/10-bad.hwdb b/test/hwdb/10-bad.hwdb
new file mode 100644 (file)
index 0000000..0e1e147
--- /dev/null
+++ b/test/hwdb/10-bad.hwdb
@@ -0,0 +1,26 @@
+BAD:1:no properties
+
+BAD:2:no properties
+BAD:2:no properties
+
+BAD:3:no properties
+BAD:3:no properties
+BAD:3:no properties
+
+GOOD:5:bad property
+ NO_VALUE
+
+GOOD:6:bad property
+ =NO_NAME
+ NO_VALUE=
+
+BAD:7:match at wrong place
+ X=Y
+BAD:7:match at wrong place
+
+BAD:8:match at wrong place
+ X=Y
+BAD:8:match at wrong place
+ Z=z
+
+BAD:8:match at EOF