From a5214bfcfefc9c14ff8b0a076afbb05cc01db767 Mon Sep 17 00:00:00 2001
From: Lev Kats <lev-katz@mail.ru>
Date: Wed, 26 Jun 2024 15:14:24 +0300
Subject: [PATCH] sio: fix error message displaying bind address
Now `sio_bind` function prints address into error message directly
instead of relying on `fd` used in `bind` that failed to execute.
`sio_bind` used `sio_socketname_to_buffer` for error message
effectively attempting printing address bound to `fd` while there
actually was an error in binding that address to that socket in the
first place.
Fixes #5925
NO_DOC=bugfix
NO_CHANGELOG=minor
---
src/lib/core/sio.c | 62 ++++++++++++++++++++++++++++++++++++----------
src/lib/core/sio.h | 10 ++++++++
test/unit/sio.c | 29 +++++++++++++++++++++-
test/unit/unit.h | 2 ++
test/unit/uri.c | 2 --
5 files changed, 89 insertions(+), 16 deletions(-)
diff --git a/src/lib/core/sio.c b/src/lib/core/sio.c
index 72f0248c84..c9bfa13b56 100644
--- a/src/lib/core/sio.c
+++ b/src/lib/core/sio.c
@@ -55,38 +55,49 @@ static_assert(SMALL_STATIC_SIZE > NI_MAXHOST + NI_MAXSERV,
* checks and all.
*/
static int
-sio_socketname_to_buffer(int fd, char *buf, int size)
+sio_socketname_to_buffer(int fd,
+ const struct sockaddr *base_addr, socklen_t addrlen,
+ const struct sockaddr *peer_addr, socklen_t peerlen,
+ char *buf, int size)
{
int n = 0;
(void)n;
SNPRINT(n, snprintf, buf, size, "fd %d", fd);
if (fd < 0)
return 0;
- struct sockaddr_storage addr;
- socklen_t addrlen = sizeof(addr);
- struct sockaddr *base_addr = (struct sockaddr *)&addr;
- int rc = getsockname(fd, base_addr, &addrlen);
- if (rc == 0) {
+ if (base_addr != NULL) {
SNPRINT(n, snprintf, buf, size, ", aka ");
SNPRINT(n, sio_addr_snprintf, buf, size, base_addr, addrlen);
}
- addrlen = sizeof(addr);
- rc = getpeername(fd, (struct sockaddr *) &addr, &addrlen);
- if (rc == 0) {
+ if (peer_addr != NULL) {
SNPRINT(n, snprintf, buf, size, ", peer of ");
- SNPRINT(n, sio_addr_snprintf, buf, size, base_addr, addrlen);
+ SNPRINT(n, sio_addr_snprintf, buf, size,
+ peer_addr, peerlen);
}
return 0;
}
const char *
-sio_socketname(int fd)
+sio_socketname_addr(int fd, const struct sockaddr *base_addr,
+ socklen_t addrlen)
{
/* Preserve errno */
int save_errno = errno;
int name_size = SERVICE_NAME_MAXLEN;
char *name = static_alloc(name_size);
- int rc = sio_socketname_to_buffer(fd, name, name_size);
+
+ struct sockaddr_storage peer_addr_storage;
+ socklen_t peerlen = sizeof(peer_addr_storage);
+ struct sockaddr *peer_addr = (struct sockaddr *)&peer_addr_storage;
+ int rcp = getpeername(fd, peer_addr, &peerlen);
+ if (rcp != 0) {
+ peer_addr = NULL;
+ }
+
+ int rc = sio_socketname_to_buffer(fd,
+ base_addr, addrlen,
+ peer_addr, peerlen,
+ name, name_size);
/*
* Could fail only because of a bad format in snprintf, but it is not
* bad, so should not fail.
@@ -101,6 +112,29 @@ sio_socketname(int fd)
return name;
}
+const char *
+sio_socketname(int fd)
+{
+ /* Preserve errno */
+ int save_errno = errno;
+
+ struct sockaddr_storage addr;
+ socklen_t addrlen = sizeof(addr);
+ struct sockaddr *base_addr = (struct sockaddr *)&addr;
+ int rcb = getsockname(fd, base_addr, &addrlen);
+ if (rcb != 0) {
+ base_addr = NULL;
+ }
+
+ const char *name = sio_socketname_addr(fd, base_addr, addrlen);
+ /*
+ * Restore the original errno, it might have been reset by
+ * getsockname().
+ */
+ errno = save_errno;
+ return name;
+}
+
/** Get a string representation of a socket option name,
* for logging.
*/
@@ -245,7 +279,9 @@ sio_bind(int fd, const struct sockaddr *addr, socklen_t addrlen)
{
int rc = bind(fd, addr, addrlen);
if (rc < 0)
- diag_set(SocketError, sio_socketname(fd), "bind");
+ diag_set(SocketError,
+ sio_socketname_addr(fd, addr, addrlen),
+ "bind");
return rc;
}
diff --git a/src/lib/core/sio.h b/src/lib/core/sio.h
index bb599fa451..1687d3403d 100644
--- a/src/lib/core/sio.h
+++ b/src/lib/core/sio.h
@@ -236,6 +236,16 @@ ssize_t sio_recvfrom(int fd, void *buf, size_t len, int flags,
int
sio_uri_to_addr(const char *uri, struct sockaddr *addr, bool *is_host_empty);
+/* internals, for unit testing */
+
+/**
+ * Return a string containing fd, base address and peer from arguments.
+ * Used mostly in error messages.
+ */
+const char *
+sio_socketname_addr(int fd, const struct sockaddr *base_addr,
+ socklen_t addrlen);
+
#if defined(__cplusplus)
} /* extern "C" */
#endif /* defined(__cplusplus) */
diff --git a/test/unit/sio.c b/test/unit/sio.c
index 79426c372b..d2fd3b1129 100644
--- a/test/unit/sio.c
+++ b/test/unit/sio.c
@@ -89,6 +89,32 @@ check_uri_to_addr(void)
footer();
}
+static void
+check_sio_socketname_addr(void)
+{
+ header();
+ plan(2);
+ struct sockaddr_storage base_storage;
+ struct sockaddr *base_addr = (struct sockaddr *)&base_storage;
+ socklen_t addrlen = sizeof(base_storage);
+
+ bool is_host_empty;
+ char base_addr_str[] = "192.0.2.255:1";
+ sio_uri_to_addr(base_addr_str, base_addr, &is_host_empty);
+
+ const char *name = sio_socketname_addr(-3,
+ base_addr, addrlen);
+
+ is_str(name, "fd -3", "works for invalid fd");
+
+ name = sio_socketname_addr(3, base_addr, addrlen);
+ char expectedbase[] = "fd 3, aka 192.0.2.255:1";
+ is_str(name, expectedbase, "works for fd and base_addr only");
+
+ check_plan();
+ footer();
+}
+
static void
check_auto_bind(void)
{
@@ -118,9 +144,10 @@ main(void)
fiber_init(fiber_c_invoke);
header();
- plan(2);
+ plan(3);
check_uri_to_addr();
check_auto_bind();
+ check_sio_socketname_addr();
int rc = check_plan();
footer();
diff --git a/test/unit/unit.h b/test/unit/unit.h
index bb0e53136d..4dd0b290a0 100644
--- a/test/unit/unit.h
+++ b/test/unit/unit.h
@@ -118,6 +118,8 @@ int check_plan(void);
#define is(a, b, ...) _ok0((a) == (b), #a " == " #b, ##__VA_ARGS__)
#define isnt(a, b, ...) _ok0((a) != (b), #a " != " #b, ##__VA_ARGS__)
+#define is_str(a, b, fmt, args...) ok(strcmp(a, b) == 0, fmt, ##args)
+
#if UNIT_TAP_COMPATIBLE
#define header() \
diff --git a/test/unit/uri.c b/test/unit/uri.c
index 674d5082bc..31ff32143e 100644
--- a/test/unit/uri.c
+++ b/test/unit/uri.c
@@ -10,8 +10,6 @@
#define URI_PARAM_MAX 10
#define URI_PARAM_VALUE_MAX 10
-#define is_str(a, b, fmt, args...) ok(strcmp(a, b) == 0, fmt, ##args)
-
static void
sample_uri_create(struct uri *uri)
{
--
GitLab