Harald Welte has submitted this change and it was merged. ( 
https://gerrit.osmocom.org/10830 )

Change subject: add osmo_str_tolower() and _toupper() with test
......................................................................

add osmo_str_tolower() and _toupper() with test

We already have osmo_str2lower() and osmo_str2upper(), but these lack:
* proper destination buffer bounds checking,
* ability to call directly as printf() argument.

Deprecate osmo_str2upper() and osmo_str2lower() because of missing bounds
checking.

Introduce osmo_str_tolower_buf(), osmo_str_toupper_buf() to provide
bounds-safe conversion, also able to safely convert a buffer in-place.

Introduce osmo_str_tolower(), osmo_str_toupper() that call the above _buf()
equivalents using a static buffer[128] and returning the resulting string
directly, convenient for direct printing. Possibly truncated but always safe.

Add unit tests to utils_test.c.

Replace all libosmocore uses of now deprecated osmo_str2lower().

Naming: the ctype.h API is called tolower() and toupper(), so just prepend
'osmo_str_' and don't separate 'to_lower'.

Change-Id: Ib0ee1206b9f31d7ba25c31f8008119ac55440797
---
M include/osmocom/core/utils.h
M src/utils.c
M src/vty/logging_vty.c
M tests/utils/utils_test.c
M tests/utils/utils_test.ok
5 files changed, 333 insertions(+), 9 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved



diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h
index 976d4a8..0b54c88 100644
--- a/include/osmocom/core/utils.h
+++ b/include/osmocom/core/utils.h
@@ -7,6 +7,7 @@
 #include <osmocom/core/backtrace.h>
 #include <osmocom/core/talloc.h>
 #include <osmocom/core/panic.h>
+#include <osmocom/core/defs.h>

 /*! \defgroup utils General-purpose utility functions
  *  @{
@@ -57,8 +58,18 @@

 #define osmo_static_assert(exp, name) typedef int dummy##name [(exp) ? 1 : -1] 
__attribute__((__unused__));

-void osmo_str2lower(char *out, const char *in);
-void osmo_str2upper(char *out, const char *in);
+void osmo_str2lower(char *out, const char *in)
+       OSMO_DEPRECATED("Use osmo_str_tolower() or osmo_str_tolower_buf() 
instead,"
+                       " to properly check target memory bounds");
+void osmo_str2upper(char *out, const char *in)
+       OSMO_DEPRECATED("Use osmo_str_toupper() or osmo_str_toupper_buf() 
instead,"
+                       " to properly check target memory bounds");
+
+size_t osmo_str_tolower_buf(char *dest, size_t dest_len, const char *src);
+const char *osmo_str_tolower(const char *src);
+
+size_t osmo_str_toupper_buf(char *dest, size_t dest_len, const char *src);
+const char *osmo_str_toupper(const char *src);

 #define OSMO_SNPRINTF_RET(ret, rem, offset, len)               \
 do {                                                           \
diff --git a/src/utils.c b/src/utils.c
index 3f40f2e..e6adcf8 100644
--- a/src/utils.c
+++ b/src/utils.c
@@ -638,4 +638,90 @@
        return g0;
 }

+/*! Convert a string to lowercase, while checking buffer size boundaries.
+ * The result written to \a dest is guaranteed to be nul terminated if \a 
dest_len > 0.
+ * If dest == src, the string is converted in-place, if necessary truncated at 
dest_len - 1 characters
+ * length as well as nul terminated.
+ * Note: similar osmo_str2lower(), but safe to use for src strings of 
arbitrary length.
+ *  \param[out] dest  Target buffer to write lowercase string.
+ *  \param[in] dest_len  Maximum buffer size of dest (e.g. sizeof(dest)).
+ *  \param[in] src  String to convert to lowercase.
+ *  \returns Length of \a src, like osmo_strlcpy(), but if \a dest == \a src 
at most \a dest_len - 1.
+ */
+size_t osmo_str_tolower_buf(char *dest, size_t dest_len, const char *src)
+{
+       size_t rc;
+       if (dest == src) {
+               if (dest_len < 1)
+                       return 0;
+               dest[dest_len - 1] = '\0';
+               rc = strlen(dest);
+       } else {
+               if (dest_len < 1)
+                       return strlen(src);
+               rc = osmo_strlcpy(dest, src, dest_len);
+       }
+       for (; *dest; dest++)
+               *dest = tolower(*dest);
+       return rc;
+}
+
+/*! Convert a string to lowercase, using a static buffer.
+ * The resulting string may be truncated if the internally used static buffer 
is shorter than src.
+ * The internal buffer is at least 128 bytes long, i.e. guaranteed to hold at 
least 127 characters and a
+ * terminating nul.
+ * See also osmo_str_tolower_buf().
+ * \param[in] src  String to convert to lowercase.
+ * \returns Resulting lowercase string in a static buffer, always nul 
terminated.
+ */
+const char *osmo_str_tolower(const char *src)
+{
+       static char buf[128];
+       osmo_str_tolower_buf(buf, sizeof(buf), src);
+       return buf;
+}
+
+/*! Convert a string to uppercase, while checking buffer size boundaries.
+ * The result written to \a dest is guaranteed to be nul terminated if \a 
dest_len > 0.
+ * If dest == src, the string is converted in-place, if necessary truncated at 
dest_len - 1 characters
+ * length as well as nul terminated.
+ * Note: similar osmo_str2upper(), but safe to use for src strings of 
arbitrary length.
+ *  \param[out] dest  Target buffer to write uppercase string.
+ *  \param[in] dest_len  Maximum buffer size of dest (e.g. sizeof(dest)).
+ *  \param[in] src  String to convert to uppercase.
+ *  \returns Length of \a src, like osmo_strlcpy(), but if \a dest == \a src 
at most \a dest_len - 1.
+ */
+size_t osmo_str_toupper_buf(char *dest, size_t dest_len, const char *src)
+{
+       size_t rc;
+       if (dest == src) {
+               if (dest_len < 1)
+                       return 0;
+               dest[dest_len - 1] = '\0';
+               rc = strlen(dest);
+       } else {
+               if (dest_len < 1)
+                       return strlen(src);
+               rc = osmo_strlcpy(dest, src, dest_len);
+       }
+       for (; *dest; dest++)
+               *dest = toupper(*dest);
+       return rc;
+}
+
+/*! Convert a string to uppercase, using a static buffer.
+ * The resulting string may be truncated if the internally used static buffer 
is shorter than src.
+ * The internal buffer is at least 128 bytes long, i.e. guaranteed to hold at 
least 127 characters and a
+ * terminating nul.
+ * See also osmo_str_toupper_buf().
+ * \param[in] src  String to convert to uppercase.
+ * \returns Resulting uppercase string in a static buffer, always nul 
terminated.
+ */
+const char *osmo_str_toupper(const char *src)
+{
+       static char buf[128];
+       osmo_str_toupper_buf(buf, sizeof(buf), src);
+       return buf;
+}
+
 /*! @} */
diff --git a/src/vty/logging_vty.c b/src/vty/logging_vty.c
index 8c8a332..7d97bb1 100644
--- a/src/vty/logging_vty.c
+++ b/src/vty/logging_vty.c
@@ -754,7 +754,6 @@
 static int config_write_log_single(struct vty *vty, struct log_target *tgt)
 {
        int i;
-       char level_lower[32];

        switch (tgt->type) {
        case LOG_TGT_TYPE_VTY:
@@ -806,21 +805,19 @@
                VTY_NEWLINE);

        /* stupid old osmo logging API uses uppercase strings... */
-       osmo_str2lower(level_lower, log_level_str(tgt->loglevel));
-       vty_out(vty, "  logging level all %s%s", level_lower, VTY_NEWLINE);
+       vty_out(vty, "  logging level all %s%s", 
osmo_str_tolower(log_level_str(tgt->loglevel)),
+               VTY_NEWLINE);

        for (i = 0; i < osmo_log_info->num_cat; i++) {
                const struct log_category *cat = &tgt->categories[i];
-               char cat_lower[32];

                /* skip empty entries in the array */
                if (!osmo_log_info->cat[i].name)
                        continue;

                /* stupid old osmo logging API uses uppercase strings... */
-               osmo_str2lower(cat_lower, osmo_log_info->cat[i].name+1);
-               osmo_str2lower(level_lower, log_level_str(cat->loglevel));
-               vty_out(vty, "  logging level %s %s%s", cat_lower, level_lower, 
VTY_NEWLINE);
+               vty_out(vty, "  logging level %s", 
osmo_str_tolower(osmo_log_info->cat[i].name+1));
+               vty_out(vty, " %s%s", 
osmo_str_tolower(log_level_str(cat->loglevel)), VTY_NEWLINE);
        }

        return 1;
diff --git a/tests/utils/utils_test.c b/tests/utils/utils_test.c
index 2f1e87d..2bb1f9c 100644
--- a/tests/utils/utils_test.c
+++ b/tests/utils/utils_test.c
@@ -616,6 +616,189 @@
        }
 }

+struct osmo_str_tolowupper_test_data {
+       const char *in;
+       bool use_static_buf;
+       size_t buflen;
+       const char *expect_lower;
+       const char *expect_upper;
+       size_t expect_rc;
+       size_t expect_rc_inplace;
+};
+
+struct osmo_str_tolowupper_test_data osmo_str_tolowupper_tests[] = {
+       {
+               .in = 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()",
+               .use_static_buf = true,
+               .expect_lower = 
"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz!@#$%^&*()",
+               .expect_upper = 
"ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()",
+       },
+       {
+               .in = 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()",
+               .buflen = 99,
+               .expect_lower = 
"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz!@#$%^&*()",
+               .expect_upper = 
"ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()",
+               .expect_rc = 62,
+               .expect_rc_inplace = 62,
+       },
+       {
+               .in = 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()",
+               .buflen = 0,
+               .expect_lower = "Unset",
+               .expect_upper = "Unset",
+               .expect_rc = 62,
+               .expect_rc_inplace = 0,
+       },
+       {
+               .in = 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()",
+               .buflen = 1,
+               .expect_lower = "",
+               .expect_upper = "",
+               .expect_rc = 62,
+               .expect_rc_inplace = 0,
+       },
+       {
+               .in = 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()",
+               .buflen = 2,
+               .expect_lower = "a",
+               .expect_upper = "A",
+               .expect_rc = 62,
+               .expect_rc_inplace = 1,
+       },
+       {
+               .in = 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()",
+               .buflen = 28,
+               .expect_lower = "abcdefghijklmnopqrstuvwxyza",
+               .expect_upper = "ABCDEFGHIJKLMNOPQRSTUVWXYZA",
+               .expect_rc = 62,
+               .expect_rc_inplace = 27,
+       },
+};
+
+
+static void osmo_str_tolowupper_test()
+{
+       int i;
+       char buf[128];
+       bool ok = true;
+       printf("\n%s\n", __func__);
+
+       for (i = 0; i < ARRAY_SIZE(osmo_str_tolowupper_tests); i++) {
+               struct osmo_str_tolowupper_test_data *d = 
&osmo_str_tolowupper_tests[i];
+               size_t rc = 0;
+               const char *res;
+
+               /* tolower */
+               if (d->use_static_buf) {
+                       res = osmo_str_tolower(d->in);
+                       printf("osmo_str_tolower(%s)\n", osmo_quote_str(d->in, 
-1));
+                       printf("               = %s\n", osmo_quote_str(res, 
-1));
+               } else {
+                       OSMO_ASSERT(sizeof(buf) >= d->buflen);
+                       osmo_strlcpy(buf, "Unset", sizeof(buf));
+                       rc = osmo_str_tolower_buf(buf, d->buflen, d->in);
+                       res = buf;
+                       printf("osmo_str_tolower_buf(%zu, %s)\n", d->buflen, 
osmo_quote_str(d->in, -1));
+                       printf("                   = %zu, %s\n", rc, 
osmo_quote_str(res, -1));
+               }
+
+               if (strcmp(res, d->expect_lower)) {
+                       printf("ERROR: osmo_str_tolowupper_test[%d] tolower\n"
+                              "       got %s\n", i, osmo_quote_str(res, -1));
+                       printf("  expected %s\n", 
osmo_quote_str(d->expect_lower, -1));
+                       ok = false;
+               }
+
+               if (!d->use_static_buf && d->expect_rc != rc) {
+                       printf("ERROR: osmo_str_tolowupper_test[%d] tolower\n"
+                              "       got rc=%zu, expected rc=%zu\n", i, rc, 
d->expect_rc);
+                       ok = false;
+               }
+
+               /* tolower, in-place */
+               if (!d->use_static_buf) {
+                       osmo_strlcpy(buf,
+                                    d->buflen ? d->in : "Unset",
+                                    sizeof(buf));
+                       rc = osmo_str_tolower_buf(buf, d->buflen, buf);
+                       res = buf;
+                       printf("osmo_str_tolower_buf(%zu, %s, in-place)\n",
+                              d->buflen, osmo_quote_str(d->in, -1));
+                       printf("                   = %zu, %s\n", rc, 
osmo_quote_str(res, -1));
+
+                       if (strcmp(res, d->expect_lower)) {
+                               printf("ERROR: osmo_str_tolowupper_test[%d] 
tolower in-place\n"
+                                      "       got %s\n", i, 
osmo_quote_str(res, -1));
+                               printf("  expected %s\n", 
osmo_quote_str(d->expect_lower, -1));
+                               ok = false;
+                       }
+
+                       if (d->expect_rc_inplace != rc) {
+                               printf("ERROR: osmo_str_tolowupper_test[%d] 
tolower in-place\n"
+                                      "       got rc=%zu, expected rc=%zu\n",
+                                      i, rc, d->expect_rc_inplace);
+                               ok = false;
+                       }
+               }
+
+               /* toupper */
+               if (d->use_static_buf) {
+                       res = osmo_str_toupper(d->in);
+                       printf("osmo_str_toupper(%s)\n", osmo_quote_str(d->in, 
-1));
+                       printf("               = %s\n", osmo_quote_str(res, 
-1));
+               } else {
+                       OSMO_ASSERT(sizeof(buf) >= d->buflen);
+                       osmo_strlcpy(buf, "Unset", sizeof(buf));
+                       rc = osmo_str_toupper_buf(buf, d->buflen, d->in);
+                       res = buf;
+                       printf("osmo_str_toupper_buf(%zu, %s)\n", d->buflen, 
osmo_quote_str(d->in, -1));
+                       printf("                   = %zu, %s\n", rc, 
osmo_quote_str(res, -1));
+               }
+
+               if (strcmp(res, d->expect_upper)) {
+                       printf("ERROR: osmo_str_tolowupper_test[%d] toupper\n"
+                              "       got %s\n", i, osmo_quote_str(res, -1));
+                       printf("  expected %s\n", 
osmo_quote_str(d->expect_upper, -1));
+                       ok = false;
+               }
+
+               if (!d->use_static_buf && d->expect_rc != rc) {
+                       printf("ERROR: osmo_str_tolowupper_test[%d] toupper\n"
+                              "       got rc=%zu, expected rc=%zu\n", i, rc, 
d->expect_rc);
+                       ok = false;
+               }
+
+               /* toupper, in-place */
+               if (!d->use_static_buf) {
+                       osmo_strlcpy(buf,
+                                    d->buflen ? d->in : "Unset",
+                                    sizeof(buf));
+                       rc = osmo_str_toupper_buf(buf, d->buflen, buf);
+                       res = buf;
+                       printf("osmo_str_toupper_buf(%zu, %s, in-place)\n",
+                              d->buflen, osmo_quote_str(d->in, -1));
+                       printf("                   = %zu, %s\n", rc, 
osmo_quote_str(res, -1));
+
+                       if (strcmp(res, d->expect_upper)) {
+                               printf("ERROR: osmo_str_tolowupper_test[%d] 
toupper in-place\n"
+                                      "       got %s\n", i, 
osmo_quote_str(res, -1));
+                               printf("  expected %s\n", 
osmo_quote_str(d->expect_upper, -1));
+                               ok = false;
+                       }
+
+                       if (d->expect_rc_inplace != rc) {
+                               printf("ERROR: osmo_str_tolowupper_test[%d] 
toupper in-place\n"
+                                      "       got rc=%zu, expected rc=%zu\n",
+                                      i, rc, d->expect_rc_inplace);
+                               ok = false;
+                       }
+               }
+       }
+
+       OSMO_ASSERT(ok);
+}
+
+
 int main(int argc, char **argv)
 {
        static const struct log_info log_info = {};
@@ -631,5 +814,6 @@
        str_quote_test();
        isqrt_test();
        osmo_sockaddr_to_str_and_uint_test();
+       osmo_str_tolowupper_test();
        return 0;
 }
diff --git a/tests/utils/utils_test.ok b/tests/utils/utils_test.ok
index abc7317..3ea8ec6 100644
--- a/tests/utils/utils_test.ok
+++ b/tests/utils/utils_test.ok
@@ -153,3 +153,49 @@
 [5] 234.23.42.123:1234 (omit addr) addr_len=0 --> :1234 rc=0
 [6] 234.23.42.123:1234 addr_len=0 --> :1234 rc=13
 [7] 234.23.42.123:1234 (omit addr) (omit port) addr_len=0 --> :0 rc=0
+
+osmo_str_tolowupper_test
+osmo_str_tolower("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+               = 
"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz!@#$%^&*()"
+osmo_str_toupper("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+               = 
"ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()"
+osmo_str_tolower_buf(99, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+                   = 62, 
"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz!@#$%^&*()"
+osmo_str_tolower_buf(99, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()", in-place)
+                   = 62, 
"abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz!@#$%^&*()"
+osmo_str_toupper_buf(99, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+                   = 62, 
"ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()"
+osmo_str_toupper_buf(99, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()", in-place)
+                   = 62, 
"ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()"
+osmo_str_tolower_buf(0, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+                   = 62, "Unset"
+osmo_str_tolower_buf(0, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()", in-place)
+                   = 0, "Unset"
+osmo_str_toupper_buf(0, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+                   = 62, "Unset"
+osmo_str_toupper_buf(0, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()", in-place)
+                   = 0, "Unset"
+osmo_str_tolower_buf(1, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+                   = 62, ""
+osmo_str_tolower_buf(1, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()", in-place)
+                   = 0, ""
+osmo_str_toupper_buf(1, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+                   = 62, ""
+osmo_str_toupper_buf(1, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()", in-place)
+                   = 0, ""
+osmo_str_tolower_buf(2, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+                   = 62, "a"
+osmo_str_tolower_buf(2, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()", in-place)
+                   = 1, "a"
+osmo_str_toupper_buf(2, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+                   = 62, "A"
+osmo_str_toupper_buf(2, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()", in-place)
+                   = 1, "A"
+osmo_str_tolower_buf(28, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+                   = 62, "abcdefghijklmnopqrstuvwxyza"
+osmo_str_tolower_buf(28, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()", in-place)
+                   = 27, "abcdefghijklmnopqrstuvwxyza"
+osmo_str_toupper_buf(28, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()")
+                   = 62, "ABCDEFGHIJKLMNOPQRSTUVWXYZA"
+osmo_str_toupper_buf(28, 
"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!@#$%^&*()", in-place)
+                   = 27, "ABCDEFGHIJKLMNOPQRSTUVWXYZA"

--
To view, visit https://gerrit.osmocom.org/10830
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib0ee1206b9f31d7ba25c31f8008119ac55440797
Gerrit-Change-Number: 10830
Gerrit-PatchSet: 3
Gerrit-Owner: Neels Hofmeyr <nhofm...@sysmocom.de>
Gerrit-Reviewer: Harald Welte <lafo...@gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)

Reply via email to