Re: pflocal socket bug

2015-09-11 Thread Svante Signell
On Fri, 2015-09-11 at 17:43 +0200, Samuel Thibault wrote:
> Svante Signell, le Fri 11 Sep 2015 17:42:19 +0200, a écrit :
> > It seems like EADDRNOTAVAIL is an appropriate error,
> 
> It is appropriate according to POSIX.
> 
> > Otherwise I can create a patch for the socklog package. WDYT?
> 
> That makes sense since POSIX allows it.

Well, the Linux manpage for connect says:
EADDRNOTAVAIL
(Internet domain sockets) The socket referred to by sockfd had not
previously been bound to an address and, upon attempting to bind it to
an ephemeral port, it was determined that all port numbers in the
ephemeral port range are currently in use. See the discussion
of /proc/sys/net/ipv4/ip_local_port_range in ip(7).
Note the comment: (Internet domain sockets), i.e. AF_INET not AF_UNIX.
ECONNREFUSED
  No-one listening on the remote address.

While POSIX says:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/connect.html
[EADDRNOTAVAIL]
The specified address is not available from the local machine.
[ECONNREFUSED]
The target address was not listening for connections or refused
the connection request.

To me the best explanation is "No-one listening on the remote address."
and "The target address was not listening for connections or refused the
connection request." Additionally it is related to connect() by the
error name itself.

The Hurd patch is very simple:
--- pflocal/socket.c~2015-09-11 19:46:33.0 +0200
+++ pflocal/socket.c2015-09-11 19:44:33.0 +0200
@@ -118,6 +118,8 @@
 return EOPNOTSUPP;
 
   err = addr_get_sock (addr, );
+  if (err == EADDRNOTAVAIL)
+err = ECONNREFUSED;--- pflocal/socket.c2015-09-11
19:46:33.0 +0200
+++ pflocal/socket.c2015-09-11 19:44:33.0 +0200
@@ -118,6 +118,8 @@
 return EOPNOTSUPP;
 
   err = addr_get_sock (addr, );
+  if (err == EADDRNOTAVAIL)
+err = ECONNREFUSED;
   if (!err)
 {
   struct sock *sock = user->sock;

   if (!err)
 {
   struct sock *sock = user->sock;

Additionally the following removes the declaration warning:
--- pflocal/sock.h~ 2015-09-11 16:28:10.0 +0200
+++ pflocal/sock.h  2015-09-11 19:28:46.0 +0200
@@ -84,6 +84,9 @@
   struct connq *connect_queue;
 };
 
+/* Bind SOCK to ADDR.  */
+error_t sock_bind (struct sock *sock, struct addr *addr);
+
 /* Socket flags */
 #define PFLOCAL_SOCK_CONNECTED 0x1 /* A connected
connection-oriented sock. */
 #define PFLOCAL_SOCK_NONBLOCK  0x2 /* Don't block on I/O.  */
@@ -154,9 +157,6 @@
 /* Return a new user port on SOCK in PORT.  */
 error_t sock_create_port (struct sock *sock, mach_port_t *port);
 
-/* Bind SOCK to ADDR.  */
-error_t sock_bind (struct sock *sock, struct addr *addr);
-
 /* Returns SOCK's address in ADDR, with an additional reference added.
If
SOCK doesn't currently have an address, one is fabricated first.  */
 error_t sock_get_addr (struct sock *sock, struct addr **addr);

Why not align to other OSes?




Re: pflocal socket bug

2015-09-11 Thread Svante Signell
On Fri, 2015-09-11 at 02:10 +0200, Samuel Thibault wrote:
> Hello,
> 
> Thanks for the detailed description and testcase!  I could fix it, this
> case was actually apparently completely untested: reference counting was
> never reaching zero, and thus the sock never actually shut down.

Nice to see such a fast solution! I think I even understood most of
it :)

So the problem was all about reference counting in socket.c
(S_socket_connect). I searched for a solution in the earlier call to
sock.c (sock_connect) in that function. Oh well! Maybe this also solves
the ssh (intermittent) problems when unlinking/removing a named socket.




[PATCH] Add support for ANSI.SYS SCP/RCP escape codes

2015-09-11 Thread James Clarke
This adds support for CSI s and u, which are equivalent to ESC 7 and 8,
saving/restoring the cursor position.

* console/display.c (handle_esc_bracket): Added support for CSI s and u.
---
 console/display.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/console/display.c b/console/display.c
index eb420fd..98c70f5 100644
--- a/console/display.c
+++ b/console/display.c
@@ -1210,6 +1210,18 @@ handle_esc_bracket (display_t display, char op)
   user->cursor.col -= (parse->params[0] ?: 1);
   limit_cursor (display);
   break;
+case 's':  /* ANSI.SYS: Save cursor and attributes.  */
+  /* Save cursor position: .  */
+  display->cursor.saved_x = user->cursor.col;
+  display->cursor.saved_y = user->cursor.row;
+  break;
+case 'u':  /* ANSI.SYS: Restore cursor and attributes.  */
+  /* Restore cursor position: .  */
+  user->cursor.col = display->cursor.saved_x;
+  user->cursor.row = display->cursor.saved_y;
+  /* In case the screen was larger before:  */
+  limit_cursor (display);
+  break;
 case 'l':
   /* Reset mode.  */
   for (i = 0; i < parse->nparams; i++)
-- 
2.5.1




Re: pflocal socket bug

2015-09-11 Thread Svante Signell
On Fri, 2015-09-11 at 10:34 +0200, Svante Signell wrote:
> On Fri, 2015-09-11 at 02:10 +0200, Samuel Thibault wrote:
> > Hello,
> > 
> > Thanks for the detailed description and testcase!  I could fix it, this
> > case was actually apparently completely untested: reference counting was
> > never reaching zero, and thus the sock never actually shut down.
> 
> Nice to see such a fast solution! I think I even understood most of
> it :)

I've tested the patched pflocal and the broken socket is reported in the
client connect() call :)

One small problem though with the socklog package:
It assumes ECONNREFUSED and now pflocal/connect reports EADDRNOTAVAIL.
It seems like EADDRNOTAVAIL is an appropriate error, but since both
Linux and pfinet reports ECONNREFUSED maybe Hurd should do that to.
Otherwise I can create a patch for the socklog package. WDYT?

> Maybe this also solves
> the ssh (intermittent) problems when unlinking/removing a named socket.

It seem like the ssh(d) delays are gone, and I'm able to log in
immediately with no minute-long timeouts :) More testing is needed
though.





Re: pflocal socket bug

2015-09-11 Thread Samuel Thibault
Svante Signell, le Fri 11 Sep 2015 17:42:19 +0200, a écrit :
> It seems like EADDRNOTAVAIL is an appropriate error,

It is appropriate according to POSIX.

> Otherwise I can create a patch for the socklog package. WDYT?

That makes sense since POSIX allows it.

Samuel