Handle corner case in using fr_sbuff_is...() functions on markers (#4839)

* Corner case fix for calling an fr_sbuff_is... function on a marker

Previously if fr_sbuff_is_...() was called on a marker and the buffer
had advanced past the end, then the function always returned false,
regardless of the character the marker was pointing at.

* Test to check fr_sbuff_is..() handles markers regardless of buffer position

* Correct parsing <num>.0 into fr_time_delta_t

diff --git a/src/lib/util/sbuff.h b/src/lib/util/sbuff.h
index 41c6e96c815e012e82a1ae0b3500c24f912c850f..881c577122cfd816cd533cfa5cc4140da32e0ac7 100644 (file)
--- a/src/lib/util/sbuff.h
+++ b/src/lib/util/sbuff.h
@@ -1729,49 +1729,58 @@ static inline bool _fr_sbuff_is_char(fr_sbuff_t *sbuff, char *p, char c)
        if (!fr_sbuff_extend(sbuff)) return false;
        return *p == c;
 }
-#define fr_sbuff_is_char(_sbuff_or_marker, _c) _fr_sbuff_is_char(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker), _c)
-
-static inline bool _fr_sbuff_is_digit(fr_sbuff_t *sbuff, char *p)
+static inline bool _fr_marker_is_char(fr_sbuff_marker_t *marker, char *p, char c)
 {
-       if (!fr_sbuff_extend(sbuff)) return false;
-       return isdigit(*p);
+       if (!fr_sbuff_extend(marker)) return false;
+       return *p == c;
 }
-#define fr_sbuff_is_digit(_sbuff_or_marker) _fr_sbuff_is_digit(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker))
+#define fr_sbuff_is_char(_sbuff_or_marker, _c) \
+       _Generic((_sbuff_or_marker), \
+                fr_sbuff_t *           : _fr_sbuff_is_char((fr_sbuff_t *)(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker), _c), \
+                fr_sbuff_marker_t *    : _fr_marker_is_char((fr_sbuff_marker_t *)(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker), _c) \
+       )
 
-static inline bool _fr_sbuff_is_upper(fr_sbuff_t *sbuff, char const *p)
-{
-       if (!fr_sbuff_extend(sbuff)) return false;
-       return isupper(*p);
-}
-#define fr_sbuff_is_upper(_sbuff_or_marker) _fr_sbuff_is_upper(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker))
+#define SBUFF_IS_FUNC(_name, _test) \
+       static inline bool _fr_sbuff_is_ ## _name(fr_sbuff_t *sbuff, char *p) \
+       { \
+               if (!fr_sbuff_extend(sbuff)) return false; \
+               return _test; \
+       }\
+       static inline bool _fr_marker_is_ ## _name(fr_sbuff_marker_t *marker, char *p) \
+       { \
+               if (!fr_sbuff_extend(marker)) return false; \
+               return _test; \
+       }
 
-static inline bool _fr_sbuff_is_lower(fr_sbuff_t *sbuff, char const *p)
-{
-       if (!fr_sbuff_extend(sbuff)) return false;
-       return islower(*p);
-}
-#define fr_sbuff_is_lower(_sbuff_or_marker) _fr_sbuff_is_lower(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker))
+#define SBUFF_IS_GENERIC(_sbuff_or_marker, _name) \
+       _Generic((_sbuff_or_marker), \
+                fr_sbuff_t *           : _fr_sbuff_is_ ## _name((fr_sbuff_t *)(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker)), \
+                fr_sbuff_marker_t *    : _fr_marker_is_ ## _name((fr_sbuff_marker_t *)(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker)) \
+       )
 
-static inline bool _fr_sbuff_is_alpha(fr_sbuff_t *sbuff, char const *p)
-{
-       if (!fr_sbuff_extend(sbuff)) return false;
-       return isalpha(*p);
-}
-#define fr_sbuff_is_alpha(_sbuff_or_marker) _fr_sbuff_is_alpha(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker))
+SBUFF_IS_FUNC(digit, isdigit(*p))
+#define fr_sbuff_is_digit(_sbuff_or_marker) \
+       SBUFF_IS_GENERIC(_sbuff_or_marker, digit)
 
-static inline bool _fr_sbuff_is_space(fr_sbuff_t *sbuff, char const *p)
-{
-       if (!fr_sbuff_extend(sbuff)) return false;
-       return isspace(*p);
-}
-#define fr_sbuff_is_space(_sbuff_or_marker) _fr_sbuff_is_space(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker))
+SBUFF_IS_FUNC(upper, isupper(*p))
+#define fr_sbuff_is_upper(_sbuff_or_marker) \
+       SBUFF_IS_GENERIC(_sbuff_or_marker, upper)
 
-static inline bool _fr_sbuff_is_hex(fr_sbuff_t *sbuff, char const *p)
-{
-       if (!fr_sbuff_extend(sbuff)) return false;
-       return isxdigit(*p);
-}
-#define fr_sbuff_is_hex(_sbuff_or_marker) _fr_sbuff_is_hex(fr_sbuff_ptr(_sbuff_or_marker), fr_sbuff_current(_sbuff_or_marker))
+SBUFF_IS_FUNC(lower, islower(*p))
+#define fr_sbuff_is_lower(_sbuff_or_marker) \
+       SBUFF_IS_GENERIC(_sbuff_or_marker, lower)
+
+SBUFF_IS_FUNC(alpha, isalpha(*p))
+#define fr_sbuff_is_alpha(_sbuff_or_marker) \
+       SBUFF_IS_GENERIC(_sbuff_or_marker, alpha)
+
+SBUFF_IS_FUNC(space, isspace(*p))
+#define fr_sbuff_is_space(_sbuff_or_marker) \
+       SBUFF_IS_GENERIC(_sbuff_or_marker, space)
+
+SBUFF_IS_FUNC(hex, isxdigit(*p))
+#define fr_sbuff_is_hex(_sbuff_or_marker) \
+       SBUFF_IS_GENERIC(_sbuff_or_marker, hex)
 
 /** @} */
 

diff --git a/src/lib/util/sbuff_tests.c b/src/lib/util/sbuff_tests.c
index be77e4af2d9235e65a19018c06219bf243b287b7..18878fa1545701088408bf76dcb010691dc4bc8a 100644 (file)
--- a/src/lib/util/sbuff_tests.c
+++ b/src/lib/util/sbuff_tests.c
@@ -72,6 +72,35 @@ static void test_parse_init(void)
        TEST_CHECK(sbuff.end == in + strlen(in));
 }
 
+static void test_is_char(void)
+{
+       char const              in[] = "i am a test string";
+       fr_sbuff_t              sbuff;
+       fr_sbuff_marker_t       marker;
+
+       fr_sbuff_init_in(&sbuff, in, sizeof(in) - 1);
+       TEST_CHECK(fr_sbuff_is_char(&sbuff, 'i'));
+       TEST_CHECK(!fr_sbuff_is_char(&sbuff, 'z'));
+
+       fr_sbuff_advance(&sbuff, 2);
+       TEST_CHECK(!fr_sbuff_is_char(&sbuff, 'i'));
+       TEST_CHECK(fr_sbuff_is_char(&sbuff, 'a'));
+
+       fr_sbuff_advance(&sbuff, 15);
+       TEST_CHECK(fr_sbuff_is_char(&sbuff, 'g'));
+       fr_sbuff_marker(&marker, &sbuff);
+       TEST_CHECK(fr_sbuff_is_char(&marker, 'g'));
+
+       /*
+        *      Ensure that after advancing the buffer past
+        *      the end, the marker can still be correctly
+        *      tested
+        */
+       fr_sbuff_advance(&sbuff, 1);
+       TEST_CHECK(!fr_sbuff_is_char(&sbuff, 'g'));
+       TEST_CHECK(fr_sbuff_is_char(&marker, 'g'));
+}
+
 static void test_bstrncpy_exact(void)
 {
        char const      in[] = "i am a test string";
@@ -1541,6 +1570,7 @@ TEST_LIST = {
         *      Basic tests
         */
        { "fr_sbuff_init",                      test_parse_init },
+       { "fr_sbuff_is_char",                   test_is_char },
        { "fr_sbuff_out_bstrncpy_exact",        test_bstrncpy_exact },
        { "fr_sbuff_out_bstrncpy",              test_bstrncpy },
        { "fr_sbuff_out_bstrncpy_allowed",      test_bstrncpy_allowed },

diff --git a/src/lib/util/time.c b/src/lib/util/time.c
index aa55024556eeb7c0580e2ea551ac4161c6b5cc7d..c27843939ebf42876c75d179ffee651cb0a36af5 100644 (file)
--- a/src/lib/util/time.c
+++ b/src/lib/util/time.c
@@ -296,7 +296,7 @@ fr_slen_t fr_time_delta_from_substr(fr_time_delta_t *out, fr_sbuff_t *in, fr_tim
        if (fr_sbuff_next_if_char(&our_in, '.')) {
                fr_sbuff_marker_t       m_f;
                size_t                  f_len;
-               uint64_t                f;      /* Fractional units */
+               uint64_t                f = 0;  /* Fractional units */
 
                /*
                 *      Normalise as a positive integer
@@ -321,7 +321,7 @@ fr_slen_t fr_time_delta_from_substr(fr_time_delta_t *out, fr_sbuff_t *in, fr_tim
                         *      up the fractional parsing when the
                         *      fraction is all zeros...
                         */
-                       if ((sberr != FR_SBUFF_PARSE_ERROR_NOT_FOUND) || fr_sbuff_is_char(&m_f, '0')) goto num_error;
+                       if ((sberr != FR_SBUFF_PARSE_ERROR_NOT_FOUND) || !fr_sbuff_is_char(&m_f, '0')) goto num_error;
                }
 
                f_len = fr_sbuff_behind(&m_f);