unix: getaddrinfo_cb status arg should be 0 or -1

The documentation in uv.h states that the status argument to the
uv_getaddrinfo() callback is either 0 or -1 but uv-unix actually
passed it the addrinfo error code. Rectify that and add a regression
test.

Fixes #754.

src/unix/getaddrinfo.c

@@ -38,22 +38,24 @@ static void uv__getaddrinfo_work(struct uv__work* w) {
 
 
 static void uv__getaddrinfo_done(struct uv__work* w, int status) {
-  uv_getaddrinfo_t* req = container_of(w, uv_getaddrinfo_t, work_req);
-  struct addrinfo *res = req->res;
-#if defined(__sun)
+  uv_getaddrinfo_t* req;
+  struct addrinfo *res;
   size_t hostlen;
 
-  if (req->hostname)
-    hostlen = strlen(req->hostname);
-  else
-    hostlen = 0;
-#endif
-
-  req->res = NULL;
-
+  req = container_of(w, uv_getaddrinfo_t, work_req);
   uv__req_unregister(req->loop, req);
 
-  /* see initialization in uv_getaddrinfo() */
+  res = req->res;
+  req->res = NULL;
+
+  (void) &hostlen;  /* Silence unused variable warning. */
+  hostlen = 0;
+#if defined(__sun)
+  if (req->hostname != NULL)
+    hostlen = strlen(req->hostname);
+#endif
+
+  /* See initialization in uv_getaddrinfo(). */
   if (req->hints)
     free(req->hints);
   else if (req->service)
@@ -67,30 +69,34 @@ static void uv__getaddrinfo_done(struct uv__work* w, int status) {
   req->service = NULL;
   req->hostname = NULL;
 
-  if (req->retcode == 0) {
-    /* OK */
-#if defined(EAI_NODATA) /* FreeBSD deprecated EAI_NODATA */
-  } else if (req->retcode == EAI_NONAME || req->retcode == EAI_NODATA) {
-#else
-  } else if (req->retcode == EAI_NONAME) {
-#endif
-    uv__set_sys_error(req->loop, ENOENT); /* FIXME compatibility hack */
-#if defined(__sun)
-  } else if (req->retcode == EAI_MEMORY && hostlen >= MAXHOSTNAMELEN) {
-    uv__set_sys_error(req->loop, ENOENT);
-#endif
-  } else {
-    req->loop->last_err.code = UV_EADDRINFO;
-    req->loop->last_err.sys_errno_ = req->retcode;
-  }
-
   if (status == -UV_ECANCELED) {
     assert(req->retcode == 0);
     req->retcode = UV_ECANCELED;
     uv__set_artificial_error(req->loop, UV_ECANCELED);
+    req->cb(req, -1, NULL);
+    return;
   }
 
-  req->cb(req, req->retcode, res);
+  if (req->retcode == 0) {
+    req->cb(req, 0, res);
+    return;
+  }
+
+  if (req->retcode == EAI_NONAME)
+    uv__set_sys_error(req->loop, ENOENT);
+#if defined(EAI_NODATA)  /* Newer FreeBSDs don't have EAI_NODATA. */
+  else if (req->retcode == EAI_NODATA)
+    uv__set_sys_error(req->loop, ENOENT);
+#elif defined(__sun)
+  if (req->retcode == EAI_MEMORY && hostlen >= MAXHOSTNAMELEN) {
+    uv__set_sys_error(req->loop, ENOENT);
+#endif
+  else {
+    req->loop->last_err.code = UV_EADDRINFO;
+    req->loop->last_err.sys_errno_ = req->retcode;
+  }
+
+  req->cb(req, -1, res);
 }
 
 

test/test-getaddrinfo.c

@@ -33,6 +33,18 @@ static int getaddrinfo_cbs = 0;
 static uv_getaddrinfo_t* getaddrinfo_handle;
 static uv_getaddrinfo_t getaddrinfo_handles[CONCURRENT_COUNT];
 static int callback_counts[CONCURRENT_COUNT];
+static int fail_cb_called;
+
+
+static void getaddrinfo_fail_cb(uv_getaddrinfo_t* req,
+                                int status,
+                                struct addrinfo* res) {
+  ASSERT(fail_cb_called == 0);
+  ASSERT(status == -1);
+  ASSERT(res == NULL);
+  uv_freeaddrinfo(res);  /* Should not crash. */
+  fail_cb_called++;
+}
 
 
 static void getaddrinfo_basic_cb(uv_getaddrinfo_t* handle,
@@ -68,6 +80,23 @@ static void getaddrinfo_cuncurrent_cb(uv_getaddrinfo_t* handle,
 }
 
 
+TEST_IMPL(getaddrinfo_fail) {
+  uv_getaddrinfo_t req;
+
+  ASSERT(0 == uv_getaddrinfo(uv_default_loop(),
+                             &req,
+                             getaddrinfo_fail_cb,
+                             "xyzzy.xyzzy.xyzzy",
+                             NULL,
+                             NULL));
+  ASSERT(0 == uv_run(uv_default_loop(), UV_RUN_DEFAULT));
+  ASSERT(fail_cb_called == 1);
+
+  MAKE_VALGRIND_HAPPY();
+  return 0;
+}
+
+
 TEST_IMPL(getaddrinfo_basic) {
   int r;
   getaddrinfo_handle = (uv_getaddrinfo_t*)malloc(sizeof(uv_getaddrinfo_t));

test/test-list.h

@@ -136,6 +136,7 @@ TEST_DECLARE   (process_title)
 TEST_DECLARE   (cwd_and_chdir)
 TEST_DECLARE   (get_memory)
 TEST_DECLARE   (hrtime)
+TEST_DECLARE   (getaddrinfo_fail)
 TEST_DECLARE   (getaddrinfo_basic)
 TEST_DECLARE   (getaddrinfo_concurrent)
 TEST_DECLARE   (getsockname_tcp)
@@ -398,6 +399,7 @@ TASK_LIST_START
 
   TEST_ENTRY  (hrtime)
 
+  TEST_ENTRY  (getaddrinfo_fail)
   TEST_ENTRY  (getaddrinfo_basic)
   TEST_ENTRY  (getaddrinfo_concurrent)
 

test/test-threadpool-cancel.c

@@ -121,7 +121,9 @@ static void getaddrinfo_cb(uv_getaddrinfo_t* req,
                            int status,
                            struct addrinfo* res) {
   ASSERT(UV_ECANCELED == uv_last_error(req->loop).code);
-  ASSERT(UV_ECANCELED == status);
+  ASSERT(status == -1);
+  ASSERT(res == NULL);
+  uv_freeaddrinfo(res);  /* Should not crash. */
   getaddrinfo_cb_called++;
 }