From 14a1dfbe108399cc7c06c242d90ddc7665f5880e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Valverde?= Date: Mon, 27 Dec 2021 13:28:29 +0000 Subject: [PATCH] wsutil/inet_addr: Refactor to use C99/POSIX types Rewrite ws_inet_pton{4,6} and ws_inet_ntop{4,6} without GLib types. Check for strerrorname_np() and use that is available, to simplify error handling. Add some minimal tests. --- ConfigureChecks.cmake | 1 + cmakeconfig.h.in | 3 ++ debian/libwsutil0.symbols | 1 + wsutil/inet_addr.c | 88 ++++++++++++++++----------------------- wsutil/inet_addr.h | 20 ++++----- wsutil/str_util.c | 14 +++++++ wsutil/str_util.h | 3 ++ wsutil/test_wsutil.c | 65 ++++++++++++++++++++++++++++- 8 files changed, 131 insertions(+), 64 deletions(-) diff --git a/ConfigureChecks.cmake b/ConfigureChecks.cmake index b52b7c3fac..dd8268e077 100644 --- a/ConfigureChecks.cmake +++ b/ConfigureChecks.cmake @@ -98,6 +98,7 @@ if(UNIX) list(APPEND CMAKE_REQUIRED_DEFINITIONS -D_GNU_SOURCE) check_symbol_exists("memmem" "string.h" HAVE_MEMMEM) check_symbol_exists("strcasestr" "string.h" HAVE_STRCASESTR) + check_symbol_exists("strerrorname_np" "string.h" HAVE_STRERRORNAME_NP) check_symbol_exists("strptime" "time.h" HAVE_STRPTIME) check_symbol_exists("vasprintf" "stdio.h" HAVE_VASPRINTF) cmake_pop_check_state() diff --git a/cmakeconfig.h.in b/cmakeconfig.h.in index 51587143cd..315c0526f5 100644 --- a/cmakeconfig.h.in +++ b/cmakeconfig.h.in @@ -256,6 +256,9 @@ /* Define if you have the 'strcasestr' function. */ #cmakedefine HAVE_STRCASESTR 1 +/* Define if you have the 'strerrorname_np' function. */ +#cmakedefine HAVE_STRERRORNAME_NP 1 + /* Define if you have the 'vasprintf' function. */ #cmakedefine HAVE_VASPRINTF 1 diff --git a/debian/libwsutil0.symbols b/debian/libwsutil0.symbols index 226096c8e2..acf149a81e 100644 --- a/debian/libwsutil0.symbols +++ b/debian/libwsutil0.symbols @@ -428,6 +428,7 @@ libwsutil.so.0 libwsutil0 #MINVER# ws_regex_pattern@Base 3.7.0 ws_socket_ptoa@Base 3.1.1 ws_strcasestr@Base 3.7.0 + ws_strerrorname_r@Base 3.7.0 ws_strptime@Base 3.7.0 ws_strtoi16@Base 2.3.0 ws_strtoi32@Base 2.3.0 diff --git a/wsutil/inet_addr.c b/wsutil/inet_addr.c index 116724d33e..19074678a3 100644 --- a/wsutil/inet_addr.c +++ b/wsutil/inet_addr.c @@ -8,8 +8,8 @@ */ #include "config.h" -#include "inet_addr.h" #define WS_LOG_DOMAIN LOG_DOMAIN_WSUTIL +#include "inet_addr.h" #include #include @@ -35,82 +35,66 @@ #define _NTOP_SRC_CAST_ #endif -#include -#include +#include "str_util.h" /* * We assume and require an inet_pton/inet_ntop that supports AF_INET * and AF_INET6. */ -static inline gboolean -_inet_pton(int af, const gchar *src, gpointer dst) +static inline bool +inet_pton_internal(int af, const char *src, void *dst, size_t dst_size, + const char *af_str) { - gint ret = inet_pton(af, src, dst); - if (G_UNLIKELY(ret < 0)) { - /* EAFNOSUPPORT */ - if (af == AF_INET) { - memset(dst, 0, sizeof(struct in_addr)); - ws_critical("ws_inet_pton4: EAFNOSUPPORT"); - } - else if (af == AF_INET6) { - memset(dst, 0, sizeof(struct in6_addr)); - ws_critical("ws_inet_pton6: EAFNOSUPPORT"); - } - else { - ws_assert_not_reached(); - } - errno = EAFNOSUPPORT; + int ret = inet_pton(af, src, dst); + if (ret < 0) { + int err = errno; + ws_log(WS_LOG_DOMAIN, LOG_LEVEL_CRITICAL, "inet_pton: %s (%d): %s", af_str, af, g_strerror(err)); + memset(dst, 0, dst_size); + errno = err; + return false; } + /* ret == 0 invalid src representation, ret == 1 success. */ return ret == 1; } -static inline const gchar * -_inet_ntop(int af, gconstpointer src, gchar *dst, guint dst_size) +static inline const char * +inet_ntop_internal(int af, const void *src, char *dst, size_t dst_size, + const char *af_str) { - const gchar *ret = inet_ntop(af, _NTOP_SRC_CAST_ src, dst, dst_size); - if (G_UNLIKELY(ret == NULL)) { - int saved_errno = errno; - gchar *errmsg = "<>"; - switch (errno) { - case EAFNOSUPPORT: - errmsg = "<>"; - ws_critical("ws_inet_ntop: EAFNOSUPPORT"); - break; - case ENOSPC: - errmsg = "<>"; - break; - default: - break; - } + const char *ret = inet_ntop(af, _NTOP_SRC_CAST_ src, dst, dst_size); + if (ret == NULL) { + int err = errno; + char errbuf[16]; + ws_log(WS_LOG_DOMAIN, LOG_LEVEL_CRITICAL, "inet_ntop: %s (%d): %s", af_str, af, g_strerror(err)); /* set result to something that can't be confused with a valid conversion */ - (void) g_strlcpy(dst, errmsg, dst_size); - /* set errno for caller */ - errno = saved_errno; + (void)g_strlcpy(dst, ws_strerrorname_r(err, errbuf, sizeof(errbuf)), dst_size); + errno = err; + return dst; } return dst; } -const gchar * -ws_inet_ntop4(gconstpointer src, gchar *dst, guint dst_size) +const char * +ws_inet_ntop4(const void *src, char *dst, size_t dst_size) { - return _inet_ntop(AF_INET, src, dst, dst_size); + return inet_ntop_internal(AF_INET, src, dst, dst_size, "AF_INET"); } -gboolean -ws_inet_pton4(const gchar *src, guint32 *dst) +bool +ws_inet_pton4(const char *src, ws_in4_addr *dst) { - return _inet_pton(AF_INET, src, dst); + return inet_pton_internal(AF_INET, src, dst, sizeof(*dst), "AF_INET"); } -const gchar * -ws_inet_ntop6(gconstpointer src, gchar *dst, guint dst_size) +const char * +ws_inet_ntop6(const void *src, char *dst, size_t dst_size) { - return _inet_ntop(AF_INET6, src, dst, dst_size); + return inet_ntop_internal(AF_INET6, src, dst, dst_size, "AF_INET6"); } -gboolean -ws_inet_pton6(const gchar *src, ws_in6_addr *dst) +bool +ws_inet_pton6(const char *src, ws_in6_addr *dst) { - return _inet_pton(AF_INET6, src, dst); + return inet_pton_internal(AF_INET6, src, dst, sizeof(*dst), "AF_INET6"); } diff --git a/wsutil/inet_addr.h b/wsutil/inet_addr.h index c7f120453f..7147c2aea5 100644 --- a/wsutil/inet_addr.h +++ b/wsutil/inet_addr.h @@ -10,10 +10,8 @@ #ifndef __WS_INET_ADDR_H__ #define __WS_INET_ADDR_H__ -#include "ws_symbol_export.h" -#include "ws_attributes.h" +#include -#include #include "inet_ipv4.h" #include "inet_ipv6.h" @@ -58,17 +56,17 @@ extern "C" { * To check for errors set errno to zero before calling ws_inet_ntop{4,6}. * ENOSPC is set if the result exceeds the given buffer size. */ -WS_DLL_PUBLIC WS_RETNONNULL const gchar * -ws_inet_ntop4(gconstpointer src, gchar *dst, guint dst_size); +WS_DLL_PUBLIC WS_RETNONNULL const char * +ws_inet_ntop4(const void *src, char *dst, size_t dst_size); -WS_DLL_PUBLIC WS_RETNONNULL const gchar * -ws_inet_ntop6(gconstpointer src, gchar *dst, guint dst_size); +WS_DLL_PUBLIC WS_RETNONNULL const char * +ws_inet_ntop6(const void *src, char *dst, size_t dst_size); -WS_DLL_PUBLIC gboolean -ws_inet_pton4(const gchar *src, ws_in4_addr *dst); +WS_DLL_PUBLIC bool +ws_inet_pton4(const char *src, ws_in4_addr *dst); -WS_DLL_PUBLIC gboolean -ws_inet_pton6(const gchar *src, ws_in6_addr *dst); +WS_DLL_PUBLIC bool +ws_inet_pton6(const char *src, ws_in6_addr *dst); #ifdef __cplusplus } diff --git a/wsutil/str_util.c b/wsutil/str_util.c index a1953baeaa..bb0ab91a0a 100644 --- a/wsutil/str_util.c +++ b/wsutil/str_util.c @@ -528,6 +528,20 @@ ws_escape_string(wmem_allocator_t *alloc, const char *string, bool add_quotes) return buf; } +const char * +ws_strerrorname_r(int errnum, char *buf, size_t buf_size) +{ +#ifdef HAVE_STRERRORNAME_NP + const char *errstr = strerrorname_np(errnum); + if (errstr != NULL) { + (void)g_strlcpy(buf, errstr, buf_size); + return buf; + } +#endif + snprintf(buf, buf_size, "Errno(%d)", errnum); + return buf; +} + /* * Editor modelines - https://www.wireshark.org/tools/modelines.html * diff --git a/wsutil/str_util.h b/wsutil/str_util.h index 110e413977..3029753c26 100644 --- a/wsutil/str_util.h +++ b/wsutil/str_util.h @@ -195,6 +195,9 @@ char *format_size_wmem(wmem_allocator_t *allocator, int64_t size, WS_DLL_PUBLIC gchar printable_char_or_period(gchar c); +WS_DLL_PUBLIC WS_RETNONNULL +const char *ws_strerrorname_r(int errnum, char *buf, size_t buf_size); + /* To pass one of two strings, singular or plural */ #define plurality(d,s,p) ((d) == 1 ? (s) : (p)) diff --git a/wsutil/test_wsutil.c b/wsutil/test_wsutil.c index a4cf0bbd9f..09c5e970da 100644 --- a/wsutil/test_wsutil.c +++ b/wsutil/test_wsutil.c @@ -12,8 +12,66 @@ #include #include -#include "str_util.h" +#include "inet_addr.h" +static void test_inet_pton4_test1(void) +{ + const char *str; + bool ok; + ws_in4_addr result, expect; + + str = "198.51.100.200"; + expect = g_htonl(3325256904); + ok = ws_inet_pton4(str, &result); + g_assert_true(ok); + g_assert_cmpint(result, ==, expect); +} + +static void test_inet_ntop4_test1(void) +{ + char result[WS_INET_ADDRSTRLEN]; + const char *expect, *ptr; + ws_in4_addr addr; + + addr = g_htonl(3325256904); + expect = "198.51.100.200"; + ptr = ws_inet_ntop4(&addr, result, sizeof(result)); + g_assert_true(ptr == result); + g_assert_cmpstr(result, ==, expect); +} + +struct in6_test { + char str[WS_INET6_ADDRSTRLEN]; + ws_in6_addr addr; +}; + +static struct in6_test in6_test1 = { + .str = "2001:db8:ffaa:ddbb:1199:2288:3377:1", + .addr = { { 0x20, 0x01, 0x0d, 0xb8, 0xff, 0xaa, 0xdd, 0xbb, + 0x11, 0x99, 0x22, 0x88, 0x33, 0x77, 0x00, 0x01 } } +}; + +static void test_inet_pton6_test1(void) +{ + bool ok; + ws_in6_addr result; + + ok = ws_inet_pton6(in6_test1.str, &result); + g_assert_true(ok); + g_assert_cmpmem(&result, sizeof(result), &in6_test1.addr, sizeof(in6_test1.addr)); +} + +static void test_inet_ntop6_test1(void) +{ + char result[WS_INET6_ADDRSTRLEN]; + const char *ptr; + + ptr = ws_inet_ntop6(&in6_test1.addr, result, sizeof(result)); + g_assert_true(ptr == result); + g_assert_cmpstr(result, ==, in6_test1.str); +} + +#include "str_util.h" static void test_format_size(void) { @@ -617,6 +675,11 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); + g_test_add_func("/inet_addr/inet_pton4", test_inet_pton4_test1); + g_test_add_func("/inet_addr/inet_ntop4", test_inet_ntop4_test1); + g_test_add_func("/inet_addr/inet_pton6", test_inet_pton6_test1); + g_test_add_func("/inet_addr/inet_ntop6", test_inet_ntop6_test1); + g_test_add_func("/str_util/format_size", test_format_size); g_test_add_func("/str_util/escape_string", test_escape_string); g_test_add_func("/str_util/strconcat", test_strconcat);