[MERGED] libosmocore[master]: osmo_sock_get_name(): clearly indicate local and remote

[Neels Hofmeyr]

Neels Hofmeyr has submitted this change and it was merged.

Change subject: osmo_sock_get_name(): clearly indicate local and remote
..


osmo_sock_get_name(): clearly indicate local and remote

In a string like

  127.0.0.1:2905<->127.0.0.1:60661

it is hard to tell which is the local part. I'd have expected it on the left,
but it is actually on the right.

To avoid doubt and bypass bikesheds on which side should be what, clearly mark
the two sides as remote and local.

  (r=127.0.0.1:2905<->l=127.0.0.1:60661)

Change-Id: I43dcc6a1906429bd0955fd7fe2eb5b8495b592d8
---
M src/socket.c
M tests/socket/socket_test.c
2 files changed, 5 insertions(+), 5 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/socket.c b/src/socket.c
index b74edd4..28d9b1c 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -607,11 +607,11 @@
if (rc < 0)
goto local_only;
 
-   return talloc_asprintf(ctx, "(%s:%s<->%s:%s)", hostbuf_r, portbuf_r,
+   return talloc_asprintf(ctx, "(r=%s:%s<->l=%s:%s)", hostbuf_r, portbuf_r,
hostbuf_l, portbuf_l);
 
 local_only:
-   return talloc_asprintf(ctx, "(NULL<->%s:%s)", hostbuf_l, portbuf_l);
+   return talloc_asprintf(ctx, "(r=NULL<->l=%s:%s)", hostbuf_l, portbuf_l);
 }
 
 #endif /* HAVE_SYS_SOCKET_H */
diff --git a/tests/socket/socket_test.c b/tests/socket/socket_test.c
index b56d50c..11ef8da 100644
--- a/tests/socket/socket_test.c
+++ b/tests/socket/socket_test.c
@@ -49,7 +49,7 @@
 * as apparently that won't work on FreeBSD if there's only one
 * address (e.g. 127.0.0.1) assigned to the entire system, like
 * the Osmocom FreeBSD build slaves */
-   OSMO_ASSERT(!strncmp(name, "(NULL<->", 7));
+   OSMO_ASSERT(!strncmp(name, "(r=NULL<->", 9));
talloc_free(name);
/* expect it to be blocking */
rc = fcntl(fd, F_GETFL);
@@ -87,7 +87,7 @@
 * as apparently that won't work on FreeBSD if there's only one
 * address (e.g. 127.0.0.1) assigned to the entire system, like
 * the Osmocom FreeBSD build slaves */
-   OSMO_ASSERT(!strncmp(name, "(NULL<->", 7));
+   OSMO_ASSERT(!strncmp(name, "(r=NULL<->", 9));
talloc_free(name);
/* expect it to be blocking */
rc = fcntl(fd, F_GETFL);
@@ -116,7 +116,7 @@
/* For some reason, on the jenkins.osmocom.org build slave with
 * FreeBSD 10 inside a jail, it fails.  Works fine on laforge's
 * FreeBSD 10 or 11 VM at home */
-   OSMO_ASSERT(!strncmp(name, "(127.0.0.1:53<->127.0.0.1", 25));
+   OSMO_ASSERT(!strncmp(name, "(r=127.0.0.1:53<->l=127.0.0.1", 29));
 #endif
talloc_free(name);
 

-- 
To view, visit https://gerrit.osmocom.org/3000
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I43dcc6a1906429bd0955fd7fe2eb5b8495b592d8
Gerrit-PatchSet: 3
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Harald Welte 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 

[ABANDON] libosmocore[master]: osmo_sock_get_name(): clearly indicate local and remote

[Neels Hofmeyr]

Neels Hofmeyr has abandoned this change.

Change subject: osmo_sock_get_name(): clearly indicate local and remote
..


Abandoned

for benefit of #3000

-- 
To view, visit https://gerrit.osmocom.org/3001
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: abandon
Gerrit-Change-Id: Id3e32de19fe07ed5b19ee5e67494926a511266b5
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 

[PATCH] libosmocore[master]: osmo_sock_get_name(): clearly indicate local and remote

[Neels Hofmeyr]

Hello Jenkins Builder,

I'd like you to reexamine a change.  Please visit

https://gerrit.osmocom.org/3000

to look at the new patch set (#2).

osmo_sock_get_name(): clearly indicate local and remote

In a string like

  127.0.0.1:2905<->127.0.0.1:60661

it is hard to tell which is the local part. I'd have expected it on the left,
but it is actually on the right.

To avoid doubt and bypass bikesheds on which side should be what, clearly mark
the two sides as remote and local.

  (r=127.0.0.1:2905<->l=127.0.0.1:60661)

Change-Id: I43dcc6a1906429bd0955fd7fe2eb5b8495b592d8
---
M src/socket.c
M tests/socket/socket_test.c
2 files changed, 5 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/00/3000/2

diff --git a/src/socket.c b/src/socket.c
index 6fa05a8..940fc6f 100644
--- a/src/socket.c
+++ b/src/socket.c
@@ -608,11 +608,11 @@
if (rc < 0)
goto local_only;
 
-   return talloc_asprintf(ctx, "(%s:%s<->%s:%s)", hostbuf_r, portbuf_r,
+   return talloc_asprintf(ctx, "(r=%s:%s<->l=%s:%s)", hostbuf_r, portbuf_r,
hostbuf_l, portbuf_l);
 
 local_only:
-   return talloc_asprintf(ctx, "(NULL<->%s:%s)", hostbuf_l, portbuf_l);
+   return talloc_asprintf(ctx, "(r=NULL<->l=%s:%s)", hostbuf_l, portbuf_l);
 }
 
 #endif /* HAVE_SYS_SOCKET_H */
diff --git a/tests/socket/socket_test.c b/tests/socket/socket_test.c
index b56d50c..11ef8da 100644
--- a/tests/socket/socket_test.c
+++ b/tests/socket/socket_test.c
@@ -49,7 +49,7 @@
 * as apparently that won't work on FreeBSD if there's only one
 * address (e.g. 127.0.0.1) assigned to the entire system, like
 * the Osmocom FreeBSD build slaves */
-   OSMO_ASSERT(!strncmp(name, "(NULL<->", 7));
+   OSMO_ASSERT(!strncmp(name, "(r=NULL<->", 9));
talloc_free(name);
/* expect it to be blocking */
rc = fcntl(fd, F_GETFL);
@@ -87,7 +87,7 @@
 * as apparently that won't work on FreeBSD if there's only one
 * address (e.g. 127.0.0.1) assigned to the entire system, like
 * the Osmocom FreeBSD build slaves */
-   OSMO_ASSERT(!strncmp(name, "(NULL<->", 7));
+   OSMO_ASSERT(!strncmp(name, "(r=NULL<->", 9));
talloc_free(name);
/* expect it to be blocking */
rc = fcntl(fd, F_GETFL);
@@ -116,7 +116,7 @@
/* For some reason, on the jenkins.osmocom.org build slave with
 * FreeBSD 10 inside a jail, it fails.  Works fine on laforge's
 * FreeBSD 10 or 11 VM at home */
-   OSMO_ASSERT(!strncmp(name, "(127.0.0.1:53<->127.0.0.1", 25));
+   OSMO_ASSERT(!strncmp(name, "(r=127.0.0.1:53<->l=127.0.0.1", 29));
 #endif
talloc_free(name);
 

-- 
To view, visit https://gerrit.osmocom.org/3000
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I43dcc6a1906429bd0955fd7fe2eb5b8495b592d8
Gerrit-PatchSet: 2
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 

libosmocore[master]: osmo_sock_get_name(): clearly indicate local and remote

[Neels Hofmeyr]

Patch Set 1: Code-Review+1

I prefer this one.

-- 
To view, visit https://gerrit.osmocom.org/3001
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id3e32de19fe07ed5b19ee5e67494926a511266b5
Gerrit-PatchSet: 1
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-HasComments: No

libosmocore[master]: osmo_sock_get_name(): clearly indicate local and remote

[Neels Hofmeyr]

Patch Set 1:

Another contestant for this change is https://gerrit.osmocom.org/3001 -- let 
the votes decide which one wins.

-- 
To view, visit https://gerrit.osmocom.org/3000
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I43dcc6a1906429bd0955fd7fe2eb5b8495b592d8
Gerrit-PatchSet: 1
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-HasComments: No