Attention is currently required from: laforge, pespin, msuraev.
Hello Jenkins Builder, laforge, pespin, msuraev,

I'd like you to do a code review. Please visit

    https://gerrit.osmocom.org/c/libosmocore/+/29123

to review the following change.


Change subject: Revert "Add osmo_sockaddr_strs_to_str()"
......................................................................

Revert "Add osmo_sockaddr_strs_to_str()"

This reverts commit e145e28a91eeca65d34d7b82caa2190fa89492b4.

Reason for revert:
The function osmo_sockaddr_strs_to_str() should not be part of the
osmo_sockaddr_str API. The implementation of this should live in
the function multiaddr_snprintf() added in patch
Icef53fe4b6e51563d97a1bc48001d67679b3b6e9
and should not use dynamic allocation.

Change-Id: I263dfd68313b896c5b474025fbca13c22ce41cdc
---
M include/osmocom/core/sockaddr_str.h
M src/sockaddr_str.c
M tests/sockaddr_str/sockaddr_str_test.c
M tests/sockaddr_str/sockaddr_str_test.ok
4 files changed, 8 insertions(+), 115 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/23/29123/1

diff --git a/include/osmocom/core/sockaddr_str.h 
b/include/osmocom/core/sockaddr_str.h
index 604f926..a46ad59 100644
--- a/include/osmocom/core/sockaddr_str.h
+++ b/include/osmocom/core/sockaddr_str.h
@@ -59,14 +59,12 @@
  *     struct osmo_sockaddr_str *my_sockaddr_str = ...;
  *     printf("got " OSMO_SOCKADDR_STR_FMT "\n", 
OSMO_SOCKADDR_STR_FMT_ARGS(my_sockaddr_str));
  */
-#define OSMO_SOCKADDR_STR_NO_PORT_FMT "%s%s%s"
-#define OSMO_SOCKADDR_STR_NO_PORT_FMT_ARGS(R) \
-       ((R) && (R)->af == AF_INET6) ? "[" : "", \
-       (R) ? (R)->ip : "NULL", \
-       ((R) && (R)->af == AF_INET6) ? "]" : ""
-
-#define OSMO_SOCKADDR_STR_FMT OSMO_SOCKADDR_STR_NO_PORT_FMT ":%u"
-#define OSMO_SOCKADDR_STR_FMT_ARGS(R) OSMO_SOCKADDR_STR_NO_PORT_FMT_ARGS(R), 
(R) ? (R)->port : 0
+#define OSMO_SOCKADDR_STR_FMT "%s%s%s:%u"
+#define OSMO_SOCKADDR_STR_FMT_ARGS(R) \
+       ((R) && (R)->af == AF_INET6)? "[" : "", \
+       (R)? (R)->ip : "NULL", \
+       ((R) && (R)->af == AF_INET6)? "]" : "", \
+       (R)? (R)->port : 0

 bool osmo_sockaddr_str_is_set(const struct osmo_sockaddr_str *sockaddr_str);
 bool osmo_sockaddr_str_is_nonzero(const struct osmo_sockaddr_str 
*sockaddr_str);
@@ -90,7 +88,6 @@
 int osmo_sockaddr_str_to_sockaddr_in(const struct osmo_sockaddr_str 
*sockaddr_str, struct sockaddr_in *dst);
 int osmo_sockaddr_str_to_sockaddr_in6(const struct osmo_sockaddr_str 
*sockaddr_str, struct sockaddr_in6 *dst);
 int osmo_sockaddr_str_to_sockaddr(const struct osmo_sockaddr_str 
*sockaddr_str, struct sockaddr_storage *dst);
-int osmo_sockaddr_strs_to_str(char *buf, size_t buf_len, const struct 
osmo_sockaddr_str **sa_str, size_t sa_str_count);

 int osmo_sockaddr_str_from_32n(struct osmo_sockaddr_str *sockaddr_str, 
uint32_t ip, uint16_t port)
        OSMO_DEPRECATED("osmo_sockaddr_str_from_32n() actually uses *host* byte 
order. Use osmo_sockaddr_str_from_32h() instead");
diff --git a/src/sockaddr_str.c b/src/sockaddr_str.c
index b5007c3..70927a7 100644
--- a/src/sockaddr_str.c
+++ b/src/sockaddr_str.c
@@ -541,48 +541,5 @@
        }
 }

-/*! Convert array of osmo_sockaddr_str address strings into a string 
representation of the address set, in the form:
- * buf_len == 0: "()"
- * buf_len == 1: "hostA"
- * buf_len >= 2: (hostA|hostB|...|...)
- * The port information is ignored.
- * \param[out] buf The buffer to store address set representation in.
- * \param[in] buf_len The size of buffer for address set representation.
- * \param[in] sa_str The array of osmo_sockaddr_str structs.
- * \param[in] sa_str_count Number of osmo_sockaddr_str structs to convert.
- * \return 0 on success, negative on error.
- */
-int osmo_sockaddr_strs_to_str(char *buf, size_t buf_len, const struct 
osmo_sockaddr_str **sa_str, size_t sa_str_count)
-{
-       size_t i;
-       struct osmo_strbuf sb = { .buf = buf, .len = buf_len };
-       char *after;
-
-       if (buf_len < 3)
-               return -ENOMEM;
-
-       if (sa_str_count != 1)
-               OSMO_STRBUF_PRINTF(sb, "(");
-
-       for (i = 0; i < sa_str_count; i++) {
-               if (sa_str_count == 1)
-                       after = "";
-               else
-                       after = (i == (sa_str_count - 1)) ? ")" : "|";
-
-               if (sa_str[i])
-                       OSMO_STRBUF_PRINTF(sb, OSMO_SOCKADDR_STR_NO_PORT_FMT 
"%s",
-                                                          
OSMO_SOCKADDR_STR_NO_PORT_FMT_ARGS(sa_str[i]), after);
-       }
-
-       if (sa_str_count == 0)
-               OSMO_STRBUF_PRINTF(sb, ")");
-
-       if (sb.chars_needed >= buf_len)
-               return -ENOSPC;
-
-       return sb.chars_needed;
-}
-
 /*! @} */
 #endif // HAVE_NETINET_IN_H
diff --git a/tests/sockaddr_str/sockaddr_str_test.c 
b/tests/sockaddr_str/sockaddr_str_test.c
index 8077b9a..0d0674b 100644
--- a/tests/sockaddr_str/sockaddr_str_test.c
+++ b/tests/sockaddr_str/sockaddr_str_test.c
@@ -24,10 +24,7 @@
 #include <errno.h>
 #include <string.h>
 #include <osmocom/core/sockaddr_str.h>
-#include <osmocom/core/application.h>
-#include <osmocom/core/logging.h>
 #include <osmocom/core/utils.h>
-#include <osmocom/core/socket.h>
 #include <netinet/in.h>

 struct osmo_sockaddr_str oip_data[] = {
@@ -262,62 +259,10 @@
        }
 }

-static int test_sockaddr_strs_dump(void *ctx, struct osmo_sockaddr_str *xdata, 
size_t count)
-{
-       char buf[OSMO_SOCK_NAME_MAXLEN * OSMO_SOCK_MAX_ADDRS * 2];
-       struct osmo_sockaddr_str *sa_strs[OSMO_SOCK_MAX_ADDRS];
-       int i, j, rc;
-
-       for (i = 0, j = 0; j < count && i < OSMO_SOCK_MAX_ADDRS; j++) 
{//ARRAY_SIZE(oip_data)
-               sa_strs[i] = talloc_zero(ctx, struct osmo_sockaddr_str);
-               struct sockaddr_storage tmp;
-               if (osmo_sockaddr_str_to_sockaddr(&xdata[j], &tmp) == 0) 
{//&oip_data[j]
-                       rc = osmo_sockaddr_str_from_sockaddr(sa_strs[i], &tmp);
-                       if (rc < 0) {
-                               printf("osmo_sockaddr_str_from_sockaddr(%d) 
failed on ", i);
-                               dump_oip(&xdata[j]);//&oip_data[j]
-                               return rc;
-                       }
-                       i++;
-               }
-       }
-       rc = osmo_sockaddr_strs_to_str(buf, sizeof(buf), (const struct 
osmo_sockaddr_str **)sa_strs, i);
-       printf("\nTest data [%d == %ld]: %s\n", rc, strlen(buf), buf);
-       return rc;
-}
-
-const struct log_info_cat default_categories[] = {
-};
-
-static struct log_info info = {
-       .cat = default_categories,
-       .num_cat = ARRAY_SIZE(default_categories),
-};
-
 int main(int argc, char **argv)
 {
-       int rc;
-       void *ctx = talloc_named_const(NULL, 0, "sockaddr_str__test");
-       osmo_init_logging2(ctx, &info);
-       log_set_use_color(osmo_stderr_target, 0);
-       log_set_print_filename2(osmo_stderr_target, LOG_FILENAME_NONE);
-       log_set_print_category(osmo_stderr_target, 0);
-       log_set_print_category_hex(osmo_stderr_target, 0);
-
        sockaddr_str_test_conversions();
        test_osmo_sockaddr_str_cmp();
-
-       rc = test_sockaddr_strs_dump(ctx, oip_data, ARRAY_SIZE(oip_data));
-       if (rc < 0)
-               return EXIT_FAILURE;
-
-       rc = test_sockaddr_strs_dump(ctx, oip_data, 1);
-       if (rc < 0)
-               return EXIT_FAILURE;
-
-       rc = test_sockaddr_strs_dump(ctx, oip_data, 0);
-       if (rc < 0)
-               return EXIT_FAILURE;
-
-       return EXIT_SUCCESS;
+       return 0;
 }
+
diff --git a/tests/sockaddr_str/sockaddr_str_test.ok 
b/tests/sockaddr_str/sockaddr_str_test.ok
index 504231c..910e919 100644
--- a/tests/sockaddr_str/sockaddr_str_test.ok
+++ b/tests/sockaddr_str/sockaddr_str_test.ok
@@ -937,9 +937,3 @@
   osmo_sockaddr_str_cmp(): [0::]:5(zero) > 0.0.0.0:5(zero)
   osmo_sockaddr_str_cmp(): [0::]:5(zero) == [::]:5(zero)
   osmo_sockaddr_str_cmp(): [0::]:5(zero) == [0::]:5(zero)
-
-Test data [179 == 179]: 
(1.2.3.4|0.0.0.0|255.255.255.255|[1:2:3::4]|[::]|[::1]|[ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff]|[ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff]|1.2.3.4|[::1:a09:807]|0.0.0.0|[::]|[::])
-
-Test data [7 == 7]: 1.2.3.4
-
-Test data [2 == 2]: ()

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

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I263dfd68313b896c5b474025fbca13c22ce41cdc
Gerrit-Change-Number: 29123
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofm...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <lafo...@osmocom.org>
Gerrit-Reviewer: msuraev <msur...@sysmocom.de>
Gerrit-Reviewer: pespin <pes...@sysmocom.de>
Gerrit-Attention: laforge <lafo...@osmocom.org>
Gerrit-Attention: pespin <pes...@sysmocom.de>
Gerrit-Attention: msuraev <msur...@sysmocom.de>
Gerrit-MessageType: newchange

Reply via email to