Re: Unix platforms shouldn't mask errors specific to Unix domain sockets

Sorry, meant to post this to the newsgroup, not directly to WTC.

-------- Original Message --------
Subject: 	Re: Unix platforms shouldn't mask errors specific to Unix 
domain sockets
Date: 	Sun, 18 Aug 2013 20:34:58 -0700
From: 	Jim Blandy <[email protected]>
To: 	Wan-Teh Chang <[email protected]>


On 08/16/2013 02:44 PM, Wan-Teh Chang wrote:
 > On Thu, Aug 15, 2013 at 5:12 PM, Jim Blandy <[email protected]> wrote:
 >>
 >> What would folks say to a patch like the attached?
 >
 > Hi Jim,
 >
 > You didn't attach a patch to your message, but I guess you want to
 > remove this case from from the _MD_unix_map_connect_error() function
 > in unix_errors.c, right?
 >
 > 531         case EACCES:
 > 532             prError = PR_ADDRESS_NOT_SUPPORTED_ERROR;
 > 533             break;
 >
 > In the latest version of the Single Unix Specification, the connect()
 > man page says EACCES can only be reported in this case:
 >
 >      The connect() function may fail if:
 >
 >      [EACCES]Search permission is denied for a component of the path
 > prefix; or write access to the named socket is denied.
 >
 > So it is more correct to map EACCES to PR_NO_ACCESS_RIGHTS_ERROR. Agreed?

(Huh. The attached patch does show up in the archives here:
https://groups.google.com/d/msg/mozilla.dev.tech.nspr/F57agalOUTw/6J9lHhVDK8UJ

I've included it directly below.)

The patch actually deletes all the switch cases in
_MD_unix_map_connect_error that map errno values to
PR_ADDRESS_NOT_SUPPORTED_ERROR. Deleting those cases has the effect of
delegating those values to _MD_unix_map_default_error, which does map
EACCES to PR_NO_ACCESS_RIGHTS_ERROR, as you suggest.

The errors this affects are those that arise only when using Unix-domain
sockets --- with the exception (that I know of) of EACCES, which Linux
will return for IP and IPv6 addresses if firewall rules forbid the
connection. Even there, I think PR_NO_ACCESS_RIGHTS_ERROR would be the
more informative status to return.

If there are NSPR clients out there that are checking for
PR_ADDRESS_NOT_SUPPORTED_ERROR while using Unix-domain sockets, they're
not going be doing very sophisticated error recovery based on those
codes. If I've thought this through right, then although the patch is
not a backwards-compatible change, it is one that will probably have
little effect beyond improving error messages.

----

Don't mask filesystem-related errors when using Unix domain sockets.
Instead, let _MD_unix_map_default_error handle them normally.

diff --git a/pr/src/md/unix/unix_errors.c b/pr/src/md/unix/unix_errors.c
--- a/pr/src/md/unix/unix_errors.c
+++ b/pr/src/md/unix/unix_errors.c
@@ -528,9 +528,6 @@ void _MD_unix_map_connect_error(int err)
      PRErrorCode prError;

      switch (err) {
-        case EACCES:
-            prError = PR_ADDRESS_NOT_SUPPORTED_ERROR;
-            break;
  #if defined(UNIXWARE)
          /*
           * On some platforms, if we connect to a port on the local host
@@ -541,12 +538,6 @@ void _MD_unix_map_connect_error(int err)
              prError = PR_CONNECT_REFUSED_ERROR;
              break;
  #endif
-        case ELOOP:
-            prError = PR_ADDRESS_NOT_SUPPORTED_ERROR;
-            break;
-        case ENOENT:
-            prError = PR_ADDRESS_NOT_SUPPORTED_ERROR;
-            break;
          case ENXIO:
              prError = PR_IO_ERROR;
              break;
@@ -565,17 +556,6 @@ void _MD_unix_map_bind_error(int err)
          case EINVAL:
              prError = PR_SOCKET_ADDRESS_IS_BOUND_ERROR;
              break;
-        /*
-         * UNIX domain sockets are not supported in NSPR
-         */
-        case EIO:
-        case EISDIR:
-        case ELOOP:
-        case ENOENT:
-        case ENOTDIR:
-        case EROFS:
-            prError = PR_ADDRESS_NOT_SUPPORTED_ERROR;
-            break;
          default:
              _MD_unix_map_default_error(err);
              return;