Return -EINVAL from _from_string() functions

We'd return -1 (-EPERM), even though we have a general rule to use real errno
values. The particular case that caught my attention was:

$ sudo udevadm control -l asdf
Failed to parse log priority 'asdf': Operation not permitted

... but "git grep 'r =.*_from_string' src/" return 110 hits. Confusingly, some
of the _from_string functions already return a proper errno value, so not all
of those are broken, but probably quite a few.

diff --git a/src/basic/string-table.c b/src/basic/string-table.c
index 116021df82bb76358092a9cea3d630ba1cd320aa..3a6376714a43ca3dd9d58ab1d7873a1f8b70c4e3 100644 (file)
--- a/src/basic/string-table.c
+++ b/src/basic/string-table.c
@@ -5,11 +5,11 @@
 
 ssize_t string_table_lookup(const char * const *table, size_t len, const char *key) {
         if (!key)
-                return -1;
+                return -EINVAL;
 
         for (size_t i = 0; i < len; ++i)
                 if (streq_ptr(table[i], key))
                         return (ssize_t) i;
 
-        return -1;
+        return -EINVAL;
 }

diff --git a/src/basic/string-table.h b/src/basic/string-table.h
index ae4ea145d39e5872f469f8cf3abea9c3305851f2..5caedacb011d1df1672db1d35998dea06c9a8b23 100644 (file)
--- a/src/basic/string-table.h
+++ b/src/basic/string-table.h
@@ -29,7 +29,7 @@ ssize_t string_table_lookup(const char * const *table, size_t len, const char *k
 #define _DEFINE_STRING_TABLE_LOOKUP_FROM_STRING_WITH_BOOLEAN(name,type,yes,scope) \
         scope type name##_from_string(const char *s) {                  \
                 if (!s)                                                 \
-                        return -1;                                      \
+                        return -EINVAL;                                 \
                 int b = parse_boolean(s);                               \
                 if (b == 0)                                             \
                         return (type) 0;                                \
@@ -60,14 +60,16 @@ ssize_t string_table_lookup(const char * const *table, size_t len, const char *k
                 unsigned u = 0;                                         \
                 type i;                                                 \
                 if (!s)                                                 \
-                        return (type) -1;                               \
+                        return -EINVAL;                                 \
                 i = (type) string_table_lookup(name##_table, ELEMENTSOF(name##_table), s); \
                 if (i >= 0)                                             \
                         return i;                                       \
-                if (safe_atou(s, &u) >= 0 && u <= max)                  \
-                        return (type) u;                                \
-                return (type) -1;                                       \
-        }                                                               \
+                if (safe_atou(s, &u) < 0)                               \
+                        return -EINVAL;                                 \
+                if (u > max)                                            \
+                        return -EINVAL;                                 \
+                return (type) u;                                        \
+        }
 
 #define _DEFINE_STRING_TABLE_LOOKUP(name,type,scope)                    \
         _DEFINE_STRING_TABLE_LOOKUP_TO_STRING(name,type,scope)          \

diff --git a/src/shared/test-tables.h b/src/shared/test-tables.h
index e68ec6049749db0dd9a99d007398a76b3ca1efb0..90210b9b598575a56c5e38bb7171ba0ba55bf2b1 100644 (file)
--- a/src/shared/test-tables.h
+++ b/src/shared/test-tables.h
@@ -33,11 +33,11 @@ static inline void _test_table(const char *name,
 
                 if (i >= 0 && i < size) {
                         if (sparse)
-                                assert_se(rev == i || rev == -1);
+                                assert_se(rev == i || rev == -EINVAL);
                         else
                                 assert_se(val != NULL && rev == i);
                 } else
-                        assert_se(val == NULL && rev == -1);
+                        assert_se(val == NULL && rev == -EINVAL);
         }
 }
 

diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
index a87cdf8747a96506f009197b545ff9f2ca294c3b..0ebef530a032084ddf6ad5985b4cf27640384bab 100644 (file)
--- a/src/test/test-process-util.c
+++ b/src/test/test-process-util.c
@@ -598,8 +598,8 @@ static void test_ioprio_class_from_to_string(void) {
         test_ioprio_class_from_to_string_one("1", 1);
         test_ioprio_class_from_to_string_one("7", 7);
         test_ioprio_class_from_to_string_one("8", 8);
-        test_ioprio_class_from_to_string_one("9", -1);
-        test_ioprio_class_from_to_string_one("-1", -1);
+        test_ioprio_class_from_to_string_one("9", -EINVAL);
+        test_ioprio_class_from_to_string_one("-1", -EINVAL);
 }
 
 static void test_setpriority_closest(void) {

diff --git a/src/test/test-rlimit-util.c b/src/test/test-rlimit-util.c
index 057ae6b2bce34e38906855e22bb2a5371bdac3e8..2ebe81df4112fffd23cf43d6f77fd9f63dbb1d73 100644 (file)
--- a/src/test/test-rlimit-util.c
+++ b/src/test/test-rlimit-util.c
@@ -53,16 +53,16 @@ int main(int argc, char *argv[]) {
         assert_se(setrlimit(RLIMIT_NOFILE, &new) >= 0);
 
         assert_se(rlimit_from_string("NOFILE") == RLIMIT_NOFILE);
-        assert_se(rlimit_from_string("LimitNOFILE") == -1);
-        assert_se(rlimit_from_string("RLIMIT_NOFILE") == -1);
-        assert_se(rlimit_from_string("xxxNOFILE") == -1);
-        assert_se(rlimit_from_string("DefaultLimitNOFILE") == -1);
+        assert_se(rlimit_from_string("LimitNOFILE") == -EINVAL);
+        assert_se(rlimit_from_string("RLIMIT_NOFILE") == -EINVAL);
+        assert_se(rlimit_from_string("xxxNOFILE") == -EINVAL);
+        assert_se(rlimit_from_string("DefaultLimitNOFILE") == -EINVAL);
 
         assert_se(rlimit_from_string_harder("NOFILE") == RLIMIT_NOFILE);
         assert_se(rlimit_from_string_harder("LimitNOFILE") == RLIMIT_NOFILE);
         assert_se(rlimit_from_string_harder("RLIMIT_NOFILE") == RLIMIT_NOFILE);
-        assert_se(rlimit_from_string_harder("xxxNOFILE") == -1);
-        assert_se(rlimit_from_string_harder("DefaultLimitNOFILE") == -1);
+        assert_se(rlimit_from_string_harder("xxxNOFILE") == -EINVAL);
+        assert_se(rlimit_from_string_harder("DefaultLimitNOFILE") == -EINVAL);
 
         for (i = 0; i < _RLIMIT_MAX; i++) {
                 _cleanup_free_ char *prefixed = NULL;