Merge pull request #22734 from poettering/decimal-str-width-test

DECIMAL_STR_WIDTH() + DECIMAL_STR_MAX() tweaks

src/basic/macro.h

@@ -309,22 +309,31 @@ static inline int __coverity_check_and_return__(int condition) {
 
 #define sizeof_field(struct_type, member) sizeof(((struct_type *) 0)->member)
 
-/* Returns the number of chars needed to format variables of the
+/* Returns the number of chars needed to format variables of the specified type as a decimal string. Adds in
- * specified type as a decimal string. Adds in extra space for a
+ * extra space for a negative '-' prefix for signed types. Includes space for the trailing NUL. */
- * negative '-' prefix (hence works correctly on signed
- * types). Includes space for the trailing NUL. */
 #define DECIMAL_STR_MAX(type)                                           \
-        (2U+(sizeof(type) <= 1 ? 3U :                                   \
+        ((size_t) IS_SIGNED_INTEGER_TYPE(type) + 1U +                   \
+            (sizeof(type) <= 1 ? 3U :                                   \
              sizeof(type) <= 2 ? 5U :                                   \
              sizeof(type) <= 4 ? 10U :                                  \
-             sizeof(type) <= 8 ? 20U : sizeof(int[-2*(sizeof(type) > 8)])))
+             sizeof(type) <= 8 ? (IS_SIGNED_INTEGER_TYPE(type) ? 19U : 20U) : sizeof(int[-2*(sizeof(type) > 8)])))
 
+/* Returns the number of chars needed to format the specified integer value. It's hence more specific than
+ * DECIMAL_STR_MAX() which answers the same question for all possible values of the specified type. Does
+ * *not* include space for a trailing NUL. (If you wonder why we special case _x_ == 0 here: it's to trick
+ * out gcc's -Wtype-limits, which would complain on comparing an unsigned type with < 0, otherwise. By
+ * special-casing == 0 here first, we can use <= 0 instead of < 0 to trick out gcc.) */
 #define DECIMAL_STR_WIDTH(x)                                      \
         ({                                                        \
                 typeof(x) _x_ = (x);                              \
-                size_t ans = IS_SIGNED_INTEGER_TYPE(_x_) ? 2 : 1; \
+                size_t ans;                                       \
+                if (_x_ == 0)                                     \
+                        ans = 1;                                  \
+                else {                                            \
+                        ans = _x_ <= 0 ? 2 : 1;                   \
                         while ((_x_ /= 10) != 0)                  \
                                 ans++;                            \
+                }                                                 \
                 ans;                                              \
         })
 

src/test/test-format-util.c

@@ -10,10 +10,12 @@ assert_cc(STRLEN("xxx") == 3);
 assert_cc(STRLEN("") == 0);
 assert_cc(STRLEN(L"xxx") == 3 * sizeof(wchar_t));
 assert_cc(STRLEN(L"") == 0);
-assert_cc(DECIMAL_STR_MAX(uint8_t) == 5);
+assert_cc(DECIMAL_STR_MAX(uint8_t) == STRLEN("255")+1);
-assert_cc(DECIMAL_STR_MAX(int8_t) == 5);
+assert_cc(DECIMAL_STR_MAX(int8_t) == STRLEN("-127")+1);
-assert_cc(DECIMAL_STR_MAX(uint64_t) == 22);
+assert_cc(DECIMAL_STR_MAX(uint64_t) == STRLEN("18446744073709551615")+1);
-assert_cc(DECIMAL_STR_MAX(char) == 5);
+assert_cc(DECIMAL_STR_MAX(int64_t) == CONST_MAX(STRLEN("-9223372036854775808"), STRLEN("9223372036854775807"))+1);
+assert_cc(DECIMAL_STR_MAX(signed char) == STRLEN("-127")+1);
+assert_cc(DECIMAL_STR_MAX(unsigned char) == STRLEN("255")+1);
 assert_cc(CONST_MAX(DECIMAL_STR_MAX(int8_t), STRLEN("xxx")) == 5);
 
 static void test_format_bytes_one(uint64_t val, bool trailing_B, const char *iec_with_p, const char *iec_without_p,

src/test/test-macro.c

@@ -340,7 +340,7 @@ TEST(flags) {
         assert_se(!FLAGS_SET(F1 | F2, F1 | F3));
         assert_se(!FLAGS_SET(F1 | F2 | F3, ~F_ALL));
 
-        // Check for no double eval.
+        /* Check for no double eval. */
         n = F2;
         f = F1;
         assert_se(!FLAGS_SET(--n, ++f));
@@ -379,7 +379,7 @@ TEST(flags) {
         assert_se(UPDATE_FLAG(F1, F_ALL, false) == 0);
         assert_se(UPDATE_FLAG(F_ALL, F_ALL, false) == 0);
 
-        // Check for no double eval.
+        /* Check for no double eval. */
         n = F2;
         f = F1;
         assert_se(UPDATE_FLAG(--n, ++f, true) == (F1 | F2));
@@ -387,4 +387,50 @@ TEST(flags) {
         assert_se(f == F2);
 }
 
+TEST(DECIMAL_STR_WIDTH) {
+        assert_se(DECIMAL_STR_WIDTH(0) == 1);
+        assert_se(DECIMAL_STR_WIDTH(1) == 1);
+        assert_se(DECIMAL_STR_WIDTH(2) == 1);
+        assert_se(DECIMAL_STR_WIDTH(9) == 1);
+        assert_se(DECIMAL_STR_WIDTH(10) == 2);
+        assert_se(DECIMAL_STR_WIDTH(11) == 2);
+        assert_se(DECIMAL_STR_WIDTH(99) == 2);
+        assert_se(DECIMAL_STR_WIDTH(100) == 3);
+        assert_se(DECIMAL_STR_WIDTH(101) == 3);
+        assert_se(DECIMAL_STR_WIDTH(-1) == 2);
+        assert_se(DECIMAL_STR_WIDTH(-2) == 2);
+        assert_se(DECIMAL_STR_WIDTH(-9) == 2);
+        assert_se(DECIMAL_STR_WIDTH(-10) == 3);
+        assert_se(DECIMAL_STR_WIDTH(-11) == 3);
+        assert_se(DECIMAL_STR_WIDTH(-99) == 3);
+        assert_se(DECIMAL_STR_WIDTH(-100) == 4);
+        assert_se(DECIMAL_STR_WIDTH(-101) == 4);
+        assert_se(DECIMAL_STR_WIDTH(UINT64_MAX) == STRLEN("18446744073709551615"));
+        assert_se(DECIMAL_STR_WIDTH(INT64_MAX) == STRLEN("9223372036854775807"));
+        assert_se(DECIMAL_STR_WIDTH(INT64_MIN) == STRLEN("-9223372036854775808"));
+}
+
+TEST(DECIMAL_STR_MAX) {
+        int8_t s8_longest = INT8_MIN;
+        int16_t s16_longest = INT16_MIN;
+        int32_t s32_longest = INT32_MIN;
+        int64_t s64_longest = INT64_MIN;
+        uint8_t u8_longest = UINT8_MAX;
+        uint16_t u16_longest = UINT16_MAX;
+        uint32_t u32_longest = UINT32_MAX;
+        uint64_t u64_longest = UINT64_MAX;
+
+        /* NB: Always add +1, because DECIMAL_STR_MAX() includes space for trailing NUL byte, but
+         * DECIMAL_STR_WIDTH() does not! */
+        assert_se(DECIMAL_STR_MAX(int8_t) == DECIMAL_STR_WIDTH(s8_longest)+1);
+        assert_se(DECIMAL_STR_MAX(int16_t) == DECIMAL_STR_WIDTH(s16_longest)+1);
+        assert_se(DECIMAL_STR_MAX(int32_t) == DECIMAL_STR_WIDTH(s32_longest)+1);
+        assert_se(DECIMAL_STR_MAX(int64_t) == DECIMAL_STR_WIDTH(s64_longest)+1);
+
+        assert_se(DECIMAL_STR_MAX(uint8_t) == DECIMAL_STR_WIDTH(u8_longest)+1);
+        assert_se(DECIMAL_STR_MAX(uint16_t) == DECIMAL_STR_WIDTH(u16_longest)+1);
+        assert_se(DECIMAL_STR_MAX(uint32_t) == DECIMAL_STR_WIDTH(u32_longest)+1);
+        assert_se(DECIMAL_STR_MAX(uint64_t) == DECIMAL_STR_WIDTH(u64_longest)+1);
+}
+
 DEFINE_TEST_MAIN(LOG_INFO);