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)