Re: [9front] ape/bsd: Enable setting the local address with bind

[Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>]

[-- Attachment #1: Type: text/plain, Size: 881 bytes --]

Hi Ori, all,

Benjamin Riefenstahl writes:
> Next for me is refining the patch.  

Attached the changes that I propose, broken down as far as I can make it. 

f0663be649266240196268bc8d157881ec6feafd ape/bsd/bind.c: Do not issue "bind *".
e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf ape/bsd/listen.c: Do not try to issue "announce *".
8714cece801fa275d804308911631e2c54988fbd ape/bsd/bind.c, ape/bsd/listen.c: Set local IP.
45126352dac3cc5020538ae4dadd6b901737c4c7 ape/bsd/bind.c: Do not pretend that the port could be negative.
7ace52e5b1637bb4d89e243f14e47d0a1ece5f82 ape/bsd/listen.c: Drop "bind 0".
7990e340626b85606111b4898c0287b93faa492d ape/bsd/bind.c, ape/bsd/listen.c: Use _syserrno.
cd8c4892cbd302a346078483175fe78a58ebe178 man/3/ip (ip): Discuss local IP addresses for "announce" and "bind".

Please tell what should be changed or if something is unclear.

so long, benny


[-- Attachment #2.1: Type: text/plain, Size: 314 bytes --]

from postmaster@9front:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: text/x-diff
	Content-Disposition: attachment;
 filename=1-f0663be649266240196268bc8d157881ec6feafd.patch

[-- Attachment #2.2: 1-f0663be649266240196268bc8d157881ec6feafd.patch.suspect --]
[-- Type: application/octet-stream, Size: 880 bytes --]

From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>
Date: Fri, 10 Jun 2022 16:02:13 +0000
Subject: [PATCH] ape/bsd/bind.c: Do not issue "bind *".


Despite the documentation, "bind *" is invalid and gives the error
"bad numeric port" in devip.c:setladdrport.

"bind 0" OTOH is actually supported fine in the Plan9 API and has the
right sematics.
---
diff 786ed6d38e1ce926ee6718b7eb4298886234714f f0663be649266240196268bc8d157881ec6feafd
--- a/sys/src/ape/lib/bsd/bind.c	Thu Feb 10 18:53:08 2022
+++ b/sys/src/ape/lib/bsd/bind.c	Fri Jun 10 18:02:13 2022
@@ -56,10 +56,7 @@
 		return -1;
 	}
 	port = _sock_inport(&r->addr);
-	if(port > 0)
-		snprintf(msg, sizeof msg, "bind %d", port);
-	else
-		strcpy(msg, "bind *");
+	snprintf(msg, sizeof msg, "bind %d", port);
 	n = write(cfd, msg, strlen(msg));
 	if(n < 0){
 		errno = EOPNOTSUPP;	/* Improve error reporting!!! */


[-- Attachment #3.1: Type: text/plain, Size: 314 bytes --]

from postmaster@9front:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: text/x-diff
	Content-Disposition: attachment;
 filename=2-e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf.patch

[-- Attachment #3.2: 2-e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf.patch.suspect --]
[-- Type: application/octet-stream, Size: 1375 bytes --]

From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>
Date: Fri, 10 Jun 2022 20:22:39 +0000
Subject: [PATCH] ape/bsd/listen.c: Do not try to issue "announce *".


"port >= 0" is always true, because the port always gets filtered
through "htons" which returns "unsigned short", so we can just drop
the "else" branch here.

Anyway "announce 0" works fine with the Plan9 API, there is not need
for "announce *" here.
---
diff f0663be649266240196268bc8d157881ec6feafd e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf
--- a/sys/src/ape/lib/bsd/listen.c	Fri Jun 10 18:02:13 2022
+++ b/sys/src/ape/lib/bsd/listen.c	Fri Jun 10 22:22:39 2022
@@ -121,7 +121,7 @@
 	int backlog;
 {
 	Rock *r;
-	int n, cfd, port;
+	int n, cfd;
 	char msg[128];
 	struct sockaddr_un *lunix;
 
@@ -139,17 +139,13 @@
 			errno = EBADF;
 			return -1;
 		}
-		port = _sock_inport(&r->addr);
-		if(port >= 0) {
-			if(write(cfd, "bind 0", 6) < 0) {
-				errno = EGREG;
-				close(cfd);
-				return -1;
-			}
-			snprintf(msg, sizeof msg, "announce %d", port);
+		/* FIXME: What is this good for? */
+		if(write(cfd, "bind 0", 6) < 0) {
+			errno = EGREG;
+			close(cfd);
+			return -1;
 		}
-		else
-			strcpy(msg, "announce *");
+		snprintf(msg, sizeof msg, "announce %d", _sock_inport(&r->addr));
 		n = write(cfd, msg, strlen(msg));
 		if(n < 0){
 			errno = EOPNOTSUPP;	/* Improve error reporting!!! */


[-- Attachment #4.1: Type: text/plain, Size: 314 bytes --]

from postmaster@9front:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: text/x-diff
	Content-Disposition: attachment;
 filename=3-8714cece801fa275d804308911631e2c54988fbd.patch

[-- Attachment #4.2: 3-8714cece801fa275d804308911631e2c54988fbd.patch.suspect --]
[-- Type: application/octet-stream, Size: 3104 bytes --]

From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>
Date: Fri, 10 Jun 2022 20:39:28 +0000
Subject: [PATCH] ape/bsd/bind.c, ape/bsd/listen.c: Set local IP.


Despite what the man pages say, local addresses can actually be set so
do that.
---
diff e3e5017aa16abfafc9fc6f7c206a2eed2a0a9ccf 8714cece801fa275d804308911631e2c54988fbd
--- a/sys/src/ape/lib/bsd/_sock_ingetaddr.c	Fri Jun 10 22:22:39 2022
+++ b/sys/src/ape/lib/bsd/_sock_ingetaddr.c	Fri Jun 10 22:39:28 2022
@@ -40,6 +40,30 @@
 }
 
 int
+_sock_inisany(int af, void *addr)
+{
+	int alen;
+	void *any;
+	/* an IPv4 address that is auto-initialized to all zeros */
+	static struct in_addr inaddr_any;
+
+	switch(af){
+	case AF_INET:
+		alen = sizeof inaddr_any.s_addr;
+		any = &inaddr_any;
+		break;
+	case AF_INET6:
+		alen = sizeof in6addr_any;
+		any = &in6addr_any;
+		break;
+	default:
+		return 0;
+	}
+
+	return 0 == memcmp(addr, any, alen);
+}
+
+int
 _sock_inaddr(int af, char *ip, char *port, void *a, int *alen)
 {
 	int len;
@@ -96,4 +120,24 @@
 		}
 		close(fd);
 	}
+}
+
+char *
+_sock_inaddr2string(Rock *r, char *dest, int dlen)
+{
+	int af = r->domain;
+	void *addr = _sock_inip(&r->addr);
+	int port = _sock_inport(&r->addr);
+	char *d = dest;
+	char *dend = dest+dlen;
+
+	if(!_sock_inisany(af, addr)){
+		inet_ntop(af, addr, d, dlen-1);
+		d = memchr(d, 0, dlen-1);
+		*(d++) = '!';
+	}
+
+	snprintf(d, dend-d, "%d", port);
+
+	return dest;
 }
--- a/sys/src/ape/lib/bsd/bind.c	Fri Jun 10 22:22:39 2022
+++ b/sys/src/ape/lib/bsd/bind.c	Fri Jun 10 22:39:28 2022
@@ -24,7 +24,7 @@
 int
 bind(int fd, void *a, int alen)
 {
-	int n, len, cfd, port;
+	int n, len, cfd;
 	struct sockaddr *sa;
 	Rock *r;
 	char msg[128];
@@ -55,17 +55,23 @@
 		errno = EBADF;
 		return -1;
 	}
-	port = _sock_inport(&r->addr);
-	snprintf(msg, sizeof msg, "bind %d", port);
+
+	strcpy(msg, "bind ");
+	_sock_inaddr2string(r, msg + 5, sizeof msg - 5);
+
 	n = write(cfd, msg, strlen(msg));
 	if(n < 0){
 		errno = EOPNOTSUPP;	/* Improve error reporting!!! */
 		close(cfd);
 		return -1;
 	}
+
 	close(cfd);
-	if(port <= 0)
+
+	if(_sock_inport(&r->addr) <= 0)
 		_sock_ingetaddr(r, &r->addr, 0, "local");
 
 	return 0;
 }
+
+
--- a/sys/src/ape/lib/bsd/listen.c	Fri Jun 10 22:22:39 2022
+++ b/sys/src/ape/lib/bsd/listen.c	Fri Jun 10 22:39:28 2022
@@ -145,7 +145,8 @@
 			close(cfd);
 			return -1;
 		}
-		snprintf(msg, sizeof msg, "announce %d", _sock_inport(&r->addr));
+		strcpy(msg, "announce ");
+		_sock_inaddr2string(r, msg + 9, sizeof msg - 9);
 		n = write(cfd, msg, strlen(msg));
 		if(n < 0){
 			errno = EOPNOTSUPP;	/* Improve error reporting!!! */
--- a/sys/src/ape/lib/bsd/priv.h	Fri Jun 10 22:22:39 2022
+++ b/sys/src/ape/lib/bsd/priv.h	Fri Jun 10 22:39:28 2022
@@ -43,5 +43,7 @@
 extern int	_sock_ipattr(char*);
 extern void*	_sock_inip(struct sockaddr*);
 extern int	_sock_inport(struct sockaddr*);
+extern int	_sock_inisany(int af, void *addr);
 extern int	_sock_inaddr(int, char*, char*, void*, int*);
 extern void	_sock_ingetaddr(Rock*, void*, int*, char*);
+extern char*	_sock_inaddr2string(Rock *r, char *dest, int dlen);


[-- Attachment #5.1: Type: text/plain, Size: 314 bytes --]

from postmaster@9front:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: text/x-diff
	Content-Disposition: attachment;
 filename=4-45126352dac3cc5020538ae4dadd6b901737c4c7.patch

[-- Attachment #5.2: 4-45126352dac3cc5020538ae4dadd6b901737c4c7.patch.suspect --]
[-- Type: application/octet-stream, Size: 624 bytes --]

From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>
Date: Sat, 11 Jun 2022 12:34:31 +0000
Subject: [PATCH] ape/bsd/bind.c: Do not pretend that the port could be negative.


Ports are filtered through "htons" which returns an "unsigned short".
---
diff 8714cece801fa275d804308911631e2c54988fbd 45126352dac3cc5020538ae4dadd6b901737c4c7
--- a/sys/src/ape/lib/bsd/bind.c	Fri Jun 10 22:39:28 2022
+++ b/sys/src/ape/lib/bsd/bind.c	Sat Jun 11 14:34:31 2022
@@ -68,7 +68,7 @@
 
 	close(cfd);
 
-	if(_sock_inport(&r->addr) <= 0)
+	if(_sock_inport(&r->addr) == 0)
 		_sock_ingetaddr(r, &r->addr, 0, "local");
 
 	return 0;


[-- Attachment #6.1: Type: text/plain, Size: 314 bytes --]

from postmaster@9front:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: text/x-diff
	Content-Disposition: attachment;
 filename=5-7ace52e5b1637bb4d89e243f14e47d0a1ece5f82.patch

[-- Attachment #6.2: 5-7ace52e5b1637bb4d89e243f14e47d0a1ece5f82.patch.suspect --]
[-- Type: application/octet-stream, Size: 773 bytes --]

From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>
Date: Wed, 06 Jul 2022 03:38:17 +0000
Subject: [PATCH] ape/bsd/listen.c: Drop "bind 0".


There does not seem to be a good reason for this.  The "bind" command
has no practical consequence.
---
diff 45126352dac3cc5020538ae4dadd6b901737c4c7 7ace52e5b1637bb4d89e243f14e47d0a1ece5f82
--- a/sys/src/ape/lib/bsd/listen.c	Sat Jun 11 14:34:31 2022
+++ b/sys/src/ape/lib/bsd/listen.c	Wed Jul  6 05:38:17 2022
@@ -139,12 +139,6 @@
 			errno = EBADF;
 			return -1;
 		}
-		/* FIXME: What is this good for? */
-		if(write(cfd, "bind 0", 6) < 0) {
-			errno = EGREG;
-			close(cfd);
-			return -1;
-		}
 		strcpy(msg, "announce ");
 		_sock_inaddr2string(r, msg + 9, sizeof msg - 9);
 		n = write(cfd, msg, strlen(msg));


[-- Attachment #7.1: Type: text/plain, Size: 314 bytes --]

from postmaster@9front:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: text/x-diff
	Content-Disposition: attachment;
 filename=6-7990e340626b85606111b4898c0287b93faa492d.patch

[-- Attachment #7.2: 6-7990e340626b85606111b4898c0287b93faa492d.patch.suspect --]
[-- Type: application/octet-stream, Size: 998 bytes --]

From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>
Date: Wed, 06 Jul 2022 03:46:07 +0000
Subject: [PATCH] ape/bsd/bind.c, ape/bsd/listen.c: Use _syserrno.

---
diff 7ace52e5b1637bb4d89e243f14e47d0a1ece5f82 7990e340626b85606111b4898c0287b93faa492d
--- a/sys/src/ape/lib/bsd/bind.c	Wed Jul  6 05:38:17 2022
+++ b/sys/src/ape/lib/bsd/bind.c	Wed Jul  6 05:46:07 2022
@@ -61,7 +61,9 @@
 
 	n = write(cfd, msg, strlen(msg));
 	if(n < 0){
-		errno = EOPNOTSUPP;	/* Improve error reporting!!! */
+		_syserrno();
+		if(errno == EPLAN9)
+			errno = EOPNOTSUPP;
 		close(cfd);
 		return -1;
 	}
--- a/sys/src/ape/lib/bsd/listen.c	Wed Jul  6 05:38:17 2022
+++ b/sys/src/ape/lib/bsd/listen.c	Wed Jul  6 05:46:07 2022
@@ -143,7 +143,9 @@
 		_sock_inaddr2string(r, msg + 9, sizeof msg - 9);
 		n = write(cfd, msg, strlen(msg));
 		if(n < 0){
-			errno = EOPNOTSUPP;	/* Improve error reporting!!! */
+			_syserrno();
+			if(errno == EPLAN9)
+				errno = EOPNOTSUPP;
 			close(cfd);
 			return -1;
 		}


[-- Attachment #8.1: Type: text/plain, Size: 314 bytes --]

from postmaster@9front:
The following attachment had content that we can't
prove to be harmless.  To avoid possible automatic
execution, we changed the content headers.
The original header was:

	Content-Type: text/x-diff
	Content-Disposition: attachment;
 filename=7-cd8c4892cbd302a346078483175fe78a58ebe178.patch

[-- Attachment #8.2: 7-cd8c4892cbd302a346078483175fe78a58ebe178.patch.suspect --]
[-- Type: application/octet-stream, Size: 2304 bytes --]

From: Benjamin Riefenstahl <b.riefenstahl@turtle-trading.net>
Date: Sat, 11 Jun 2022 13:00:05 +0000
Subject: [PATCH] man/3/ip (ip): Discuss local IP addresses for "announce" and "bind".

---
diff 7990e340626b85606111b4898c0287b93faa492d cd8c4892cbd302a346078483175fe78a58ebe178
--- a/sys/man/3/ip	Wed Jul  6 05:46:07 2022
+++ b/sys/man/3/ip	Sat Jun 11 15:00:05 2022
@@ -682,33 +682,43 @@
 The connect fails if the combination of local and remote address/port pairs
 are already assigned to another port.
 .TP
-.BI announce\  X
-.I X
-is a decimal port number or
-.LR * .
-Set the local port
-number to
-.I X
-and accept calls to
-.IR X .
-If
-.I X
-is
+.BI announce\  [ip-address ! ]port
+Set the local IP address and port number and accept calls there.  If
+ip-address is left out, accept calls on any address.  If port is 0, a
+port is automatically choosen that is not yet announced.
+If the address is
 .LR * ,
-accept
-calls for any port that no process has explicitly announced.
-The local IP address cannot be set.
+accept calls on any address.
+If port is
+.LR * ,
+accept calls on any port.
+If port is
+.LR * ,
+and the address is left out, accept calls on any address and port.
 .B Announce
-fails if the connection is already announced or connected.
+fails if the connection is already announced.
 .TP
-.BI bind\  X
-.I X
-is a decimal port number or
-.LR * .
-Set the local port number to
-.IR X .
-This exists to support emulation
-of BSD sockets by the APE libraries (see
+.BI bind\  [ip-address ! ]port
+Set the local IP address and port number like for a server connection
+similar to the
+.B announce
+command.
+If ip-address is left out, an address is automatically selected.  If
+port is 0, a port is automatically choosen that is not yet announced.
+This command has no actual effect, beyond remembering the parameters
+and possibly selecting an unused port.
+The commands
+.B announce
+and
+.B connect
+reset both the local address and the port according to their own
+parameters.
+This command also does
+.B not
+reserve the IP address and port, another connection can use them, even
+while they are registered in a connection by this command.
+This exists to support emulation of BSD sockets by the APE libraries
+(see
 .IR pcc (1))
 and is not otherwise used.
 .\" this is gone