From 823073aa911bbc218804774e6bfe9ad80310596c Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Mon, 28 Oct 2019 04:58:04 +0100 Subject: utils.h: add OSMO_NAME_C_IMPL() macro Provide a common implementation for foo_name_c() functions that base on foo_name_buf() functions. char *foo_name_c(void *ctx, example_t arg) { OSMO_NAME_C_IMPL(ctx, 64, "ERROR", foo_name_buf, arg) } Rationale: the most efficient way of composing strings that have optional parts or require loops for composition is by writing to a ready char[], and this in turn is easiest done by using OSMO_STRBUF_* API. Using such a basic name string implementation which typically returns a length, I often want a more convenient version that returns a char*, which can just be inlined in a "%s" string format -- crucially: skipping string composition when inlined in a LOGP(). This common implementation allows saving code dup, only the function signature is needed. Why not include the function signature in the macro? The two sets of varargs (1: signature args, 2: function call args) are hard to do. Also, having an explicit signature is good for readability and code grepping / ctags. Upcoming uses: in libosmocore in the mslookup (D-GSM) implementation (osmo_mslookup_result_name_c()), and in osmo_msc's codec negotiation implementation (sdp_audio_codecs_name_c(), sdp_msg_name_c(), ...). I54b6c0810f181259da307078977d9ef3d90458c9 (libosmocore) If3ce23cd5bab15e2ab4c52ef3e4c75979dffe931 (osmo-msc) Change-Id: Ida5ba8d9640ea641aafef0236800f6d489d3d322 --- include/osmocom/core/utils.h | 56 ++++++++++++++++++++++++++++++ tests/utils/utils_test.c | 81 ++++++++++++++++++++++++++++++++++++++++++++ tests/utils/utils_test.ok | 17 ++++++++++ 3 files changed, 154 insertions(+) diff --git a/include/osmocom/core/utils.h b/include/osmocom/core/utils.h index c4e6f5fc..86d45bcc 100644 --- a/include/osmocom/core/utils.h +++ b/include/osmocom/core/utils.h @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -271,4 +272,59 @@ struct osmo_strbuf { bool osmo_str_startswith(const char *str, const char *startswith_str); +/*! Translate a buffer function to a talloc context function. + * This is the full function body of a char *foo_name_c(void *ctx, val...) function, implemented by an + * int foo_name_buf(buf, buflen, val...) function: + * + * char *foo_name_c(void *ctx, example_t arg) + * { + * OSMO_NAME_C_IMPL(ctx, 64, "ERROR", foo_name_buf, arg) + * } + * + * Return a talloc'd string containing the result of the given foo_name_buf() function, or ON_ERROR on error in the called + * foo_name_buf() function. + * + * If ON_ERROR is NULL, the function returns NULL on error rc from FUNC_BUF. Take care: returning NULL in printf() like + * formats (LOGP()) makes the program crash. If ON_ERROR is non-NULL, it must be a string constant, which is not + * returned directly, but written to an allocated string buffer first. + * + * \param[in] INITIAL_BUFSIZE Which size to first talloc from ctx -- a larger size makes a reallocation less likely, a + * smaller size allocates less unused bytes, zero allocates once but still runs the string composition twice. + * \param[in] ON_ERROR String constant to copy on error rc returned by FUNC_BUF, or NULL to return NULL. + * \param[in] FUNC_BUF Name of a function with signature foo_buf(char *buf, size_t buflen, ...). + * \param[in] FUNC_BUF_ARGS Additional arguments to pass to FUNC_BUF after the buf and buflen. + */ +#define OSMO_NAME_C_IMPL(CTX, INITIAL_BUFSIZE, ON_ERROR, FUNC_BUF, FUNC_BUF_ARGS...) \ + size_t _len = INITIAL_BUFSIZE; \ + int _needed; \ + char *_str = NULL; \ + if ((INITIAL_BUFSIZE) > 0) { \ + _str = (char*)talloc_named_const(CTX, _len, __func__); \ + OSMO_ASSERT(_str); \ + } \ + _needed = FUNC_BUF(_str, _len, ## FUNC_BUF_ARGS); \ + if (_needed < 0) \ + goto OSMO_NAME_C_on_error; \ + if (_needed < _len) \ + return _str; \ + _len = _needed + 1; \ + if (_str) \ + talloc_free(_str); \ + _str = (char*)talloc_named_const(CTX, _len, __func__); \ + OSMO_ASSERT(_str); \ + _needed = FUNC_BUF(_str, _len, ## FUNC_BUF_ARGS); \ + if (_needed < 0) \ + goto OSMO_NAME_C_on_error; \ + return _str; \ +OSMO_NAME_C_on_error: \ + /* Re-using and re-sizing above allocated buf ends up in very complex code. Just free and strdup. */ \ + if (_str) \ + talloc_free(_str); \ + if (!(ON_ERROR)) \ + return NULL; \ + _str = talloc_strdup(CTX, ON_ERROR); \ + OSMO_ASSERT(_str); \ + talloc_set_name_const(_str, __func__); \ + return _str; + /*! @} */ diff --git a/tests/utils/utils_test.c b/tests/utils/utils_test.c index 70d017fe..55c9e7f7 100644 --- a/tests/utils/utils_test.c +++ b/tests/utils/utils_test.c @@ -1058,6 +1058,86 @@ static void startswith_test() startswith_test_str("abc", "xyz", false); } +static int foo_name_buf(char *buf, size_t buflen, const char *arg) +{ + if (!arg) + return -EINVAL; + return snprintf(buf, buflen, "%s", arg); +} + +static char *foo_name_c(void *ctx, const char *arg) +{ + OSMO_NAME_C_IMPL(ctx, 10, "ERROR", foo_name_buf, arg) +} + +static char *foo_name_c_null(void *ctx, const char *arg) +{ + OSMO_NAME_C_IMPL(ctx, 10, NULL, foo_name_buf, arg) +} + +static char *foo_name_c_zero(void *ctx, const char *arg) +{ + OSMO_NAME_C_IMPL(ctx, 0, "ERROR", foo_name_buf, arg) +} + +static char *foo_name_c_zero_null(void *ctx, const char *arg) +{ + OSMO_NAME_C_IMPL(ctx, 0, NULL, foo_name_buf, arg) +} + +static void name_c_impl_test() +{ + char *test_strs[] = { + "test", + "longer than 10 chars", + NULL, + }; + struct { + const char *label; + char *(*func)(void *, const char*); + } funcs[] = { + { + "OSMO_NAME_C_IMPL(10, \"ERROR\")", + foo_name_c, + }, + { + "OSMO_NAME_C_IMPL(10, NULL)", + foo_name_c_null, + }, + { + "OSMO_NAME_C_IMPL(0, \"ERROR\")", + foo_name_c_zero, + }, + { + "OSMO_NAME_C_IMPL(0, NULL)", + foo_name_c_zero_null, + }, + }; + + int i; + void *ctx = talloc_named_const(NULL, 0, __func__); + int allocs = talloc_total_blocks(ctx); + + printf("\n%s\n", __func__); + for (i = 0; i < ARRAY_SIZE(test_strs); i++) { + char *test_str = test_strs[i]; + int j; + printf("%2d: %s\n", i, osmo_quote_str(test_str, -1)); + + for (j = 0; j < ARRAY_SIZE(funcs); j++) { + char *str = funcs[j].func(ctx, test_str); + printf(" %30s -> %s", funcs[j].label, osmo_quote_str(str, -1)); + printf(" allocated %d", (int)talloc_total_blocks(ctx) - allocs); + if (str) { + printf(" %zu bytes, name '%s'", talloc_total_size(str), talloc_get_name(str)); + talloc_free(str); + } + printf("\n"); + } + } + talloc_free(ctx); +} + int main(int argc, char **argv) { static const struct log_info log_info = {}; @@ -1078,5 +1158,6 @@ int main(int argc, char **argv) strbuf_test(); strbuf_test_nolen(); startswith_test(); + name_c_impl_test(); return 0; } diff --git a/tests/utils/utils_test.ok b/tests/utils/utils_test.ok index c150a8d0..b6036476 100644 --- a/tests/utils/utils_test.ok +++ b/tests/utils/utils_test.ok @@ -360,3 +360,20 @@ osmo_str_startswith("abc", "ab") == true osmo_str_startswith("abc", "abc") == true osmo_str_startswith("abc", "abcd") == false osmo_str_startswith("abc", "xyz") == false + +name_c_impl_test + 0: "test" + OSMO_NAME_C_IMPL(10, "ERROR") -> "test" allocated 1 10 bytes, name 'foo_name_c' + OSMO_NAME_C_IMPL(10, NULL) -> "test" allocated 1 10 bytes, name 'foo_name_c_null' + OSMO_NAME_C_IMPL(0, "ERROR") -> "test" allocated 1 5 bytes, name 'foo_name_c_zero' + OSMO_NAME_C_IMPL(0, NULL) -> "test" allocated 1 5 bytes, name 'foo_name_c_zero_null' + 1: "longer than 10 chars" + OSMO_NAME_C_IMPL(10, "ERROR") -> "longer than 10 chars" allocated 1 21 bytes, name 'foo_name_c' + OSMO_NAME_C_IMPL(10, NULL) -> "longer than 10 chars" allocated 1 21 bytes, name 'foo_name_c_null' + OSMO_NAME_C_IMPL(0, "ERROR") -> "longer than 10 chars" allocated 1 21 bytes, name 'foo_name_c_zero' + OSMO_NAME_C_IMPL(0, NULL) -> "longer than 10 chars" allocated 1 21 bytes, name 'foo_name_c_zero_null' + 2: NULL + OSMO_NAME_C_IMPL(10, "ERROR") -> "ERROR" allocated 1 6 bytes, name 'foo_name_c' + OSMO_NAME_C_IMPL(10, NULL) -> NULL allocated 0 + OSMO_NAME_C_IMPL(0, "ERROR") -> "ERROR" allocated 1 6 bytes, name 'foo_name_c_zero' + OSMO_NAME_C_IMPL(0, NULL) -> NULL allocated 0 -- cgit v1.2.3