]> git.ipfire.org Git - thirdparty/curl.git/commitdiff
multi: fix curl_multi_waitfds reporting of fd_count 15155/head
authorChristopher Dannemiller <cdannemiller@nvidia.com>
Fri, 4 Oct 2024 16:31:59 +0000 (09:31 -0700)
committerJay Satiro <raysatiro@yahoo.com>
Sun, 29 Dec 2024 06:05:09 +0000 (01:05 -0500)
- Make curl_multi_waitfds consistent with the documentation.

Issue Addressed:

 - The documentation of curl_multi_waitfds indicates that users should
   be able to call curl_multi_waitfds with a NULL ufds. However, before
   this change, the function would return CURLM_BAD_FUNCTION_ARGUMENT.
 - Additionally, the documentation suggests that users can use this
   function to determine the number of file descriptors (fds) needed.
   However, the function would stop counting fds if the supplied fds
   were exhausted.

Changes Made:

 - NULL ufds Handling: curl_multi_waitfds can now accept a NULL ufds if
   size is also zero.
 - Counting File Descriptors: If curl_multi_waitfds is passed a NULL
   ufds, or the size of ufds is insufficient, the output parameter
   fd_count will return the number of fds needed. This value may be
   higher than actually needed but never lower.

Testing:

 - Test 2405 has been updated to cover the usage scenarios described
   above.

Fixes https://github.com/curl/curl/issues/15146
Closes https://github.com/curl/curl/pull/15155

docs/libcurl/curl_multi_waitfds.md
lib/conncache.c
lib/conncache.h
lib/multi.c
lib/select.c
lib/select.h
tests/libtest/lib2405.c

index 0a8440eb4076650d6624094e87966dfbdad1b68b..e2ebd374124c11423baf0ff9c6ecfa4241d78410 100644 (file)
@@ -45,12 +45,13 @@ If a number of descriptors used by the multi_handle is greater than the
 *size* parameter then libcurl returns CURLM_OUT_OF_MEMORY error.
 
 If the *fd_count* argument is not a null pointer, it points to a variable
-that on returns specifies the number of descriptors used by the multi_handle to
+that on return specifies the number of descriptors used by the multi_handle to
 be checked for being ready to read or write.
 
 The client code can pass *size* equal to zero just to get the number of the
 descriptors and allocate appropriate storage for them to be used in a
-subsequent function call.
+subsequent function call. In this case, *fd_count* receives a number greater
+than or equal to the number of descriptors.
 
 # %PROTOCOLS%
 
@@ -89,7 +90,7 @@ int main(void)
     ufds = (struct curl_waitfd*)malloc(fd_count * sizeof(struct curl_waitfd));
 
     /* get wait descriptors from the transfers and put them into array. */
-    mc = curl_multi_waitfds(multi, ufds, fd_count, NULL);
+    mc = curl_multi_waitfds(multi, ufds, fd_count, &fd_count);
 
     if(mc != CURLM_OK) {
       fprintf(stderr, "curl_multi_waitfds() failed, code %d.\n", mc);
index 589205bc15297f3b9918c20c24ff424c47d41f90..61000ce75b55bdde5277ddc6e9fb7926e7969f3f 100644 (file)
@@ -926,10 +926,10 @@ CURLcode Curl_cpool_add_pollfds(struct cpool *cpool,
   return result;
 }
 
-CURLcode Curl_cpool_add_waitfds(struct cpool *cpool,
-                                struct curl_waitfds *cwfds)
+unsigned int Curl_cpool_add_waitfds(struct cpool *cpool,
+                                    struct curl_waitfds *cwfds)
 {
-  CURLcode result = CURLE_OK;
+  unsigned int need = 0;
 
   CPOOL_LOCK(cpool);
   if(Curl_llist_head(&cpool->shutdowns)) {
@@ -945,14 +945,11 @@ CURLcode Curl_cpool_add_waitfds(struct cpool *cpool,
       Curl_conn_adjust_pollset(cpool->idata, &ps);
       Curl_detach_connection(cpool->idata);
 
-      result = Curl_waitfds_add_ps(cwfds, &ps);
-      if(result)
-        goto out;
+      need += Curl_waitfds_add_ps(cwfds, &ps);
     }
   }
-out:
   CPOOL_UNLOCK(cpool);
-  return result;
+  return need;
 }
 
 static void cpool_perform(struct cpool *cpool)
index d98feffb3efe7cad0d2d2c4966b15f722aac428c..5a1bf10959ffb859718170369696ec377b1ef653 100644 (file)
@@ -183,8 +183,8 @@ void Curl_cpool_do_locked(struct Curl_easy *data,
  */
 CURLcode Curl_cpool_add_pollfds(struct cpool *connc,
                                 struct curl_pollfds *cpfds);
-CURLcode Curl_cpool_add_waitfds(struct cpool *connc,
-                                struct curl_waitfds *cwfds);
+unsigned int Curl_cpool_add_waitfds(struct cpool *connc,
+                                    struct curl_waitfds *cwfds);
 
 /**
  * Perform maintenance on connections in the pool. Specifically,
index 9d76ae9696caebd36e53c93baf027e3fe81ee050..94e64478cd1d5588aa9a8dc30e0d31f64775efc9 100644 (file)
@@ -1200,8 +1200,9 @@ CURLMcode curl_multi_waitfds(CURLM *m,
   CURLMcode result = CURLM_OK;
   struct Curl_llist_node *e;
   struct Curl_multi *multi = m;
+  unsigned int need = 0;
 
-  if(!ufds)
+  if(!ufds && (size || !fd_count))
     return CURLM_BAD_FUNCTION_ARGUMENT;
 
   if(!GOOD_MULTI_HANDLE(multi))
@@ -1214,20 +1215,17 @@ CURLMcode curl_multi_waitfds(CURLM *m,
   for(e = Curl_llist_head(&multi->process); e; e = Curl_node_next(e)) {
     struct Curl_easy *data = Curl_node_elem(e);
     multi_getsock(data, &data->last_poll);
-    if(Curl_waitfds_add_ps(&cwfds, &data->last_poll)) {
-      result = CURLM_OUT_OF_MEMORY;
-      goto out;
-    }
+    need += Curl_waitfds_add_ps(&cwfds, &data->last_poll);
   }
 
-  if(Curl_cpool_add_waitfds(&multi->cpool, &cwfds)) {
+  need += Curl_cpool_add_waitfds(&multi->cpool, &cwfds);
+
+  if(need != cwfds.n && ufds) {
     result = CURLM_OUT_OF_MEMORY;
-    goto out;
   }
 
-out:
   if(fd_count)
-    *fd_count = cwfds.n;
+    *fd_count = need;
   return result;
 }
 
index ab11e0871d2f800060861ad980ddd098987f6fdd..a0f2ade071b19b980450cf6a75545455e3506301 100644 (file)
@@ -493,14 +493,14 @@ void Curl_waitfds_init(struct curl_waitfds *cwfds,
                        unsigned int static_count)
 {
   DEBUGASSERT(cwfds);
-  DEBUGASSERT(static_wfds);
+  DEBUGASSERT(static_wfds || !static_count);
   memset(cwfds, 0, sizeof(*cwfds));
   cwfds->wfds = static_wfds;
   cwfds->count = static_count;
 }
 
-static CURLcode cwfds_add_sock(struct curl_waitfds *cwfds,
-                               curl_socket_t sock, short events)
+static unsigned int cwfds_add_sock(struct curl_waitfds *cwfds,
+                                   curl_socket_t sock, short events)
 {
   int i;
 
@@ -508,23 +508,24 @@ static CURLcode cwfds_add_sock(struct curl_waitfds *cwfds,
     for(i = (int)cwfds->n - 1; i >= 0; --i) {
       if(sock == cwfds->wfds[i].fd) {
         cwfds->wfds[i].events |= events;
-        return CURLE_OK;
+        return 0;
       }
     }
   }
   /* not folded, add new entry */
-  if(cwfds->n >= cwfds->count)
-    return CURLE_OUT_OF_MEMORY;
-  cwfds->wfds[cwfds->n].fd = sock;
-  cwfds->wfds[cwfds->n].events = events;
-  ++cwfds->n;
-  return CURLE_OK;
+  if(cwfds->n < cwfds->count) {
+    cwfds->wfds[cwfds->n].fd = sock;
+    cwfds->wfds[cwfds->n].events = events;
+    ++cwfds->n;
+  }
+  return 1;
 }
 
-CURLcode Curl_waitfds_add_ps(struct curl_waitfds *cwfds,
-                             struct easy_pollset *ps)
+unsigned int Curl_waitfds_add_ps(struct curl_waitfds *cwfds,
+                                 struct easy_pollset *ps)
 {
   size_t i;
+  unsigned int need = 0;
 
   DEBUGASSERT(cwfds);
   DEBUGASSERT(ps);
@@ -534,10 +535,8 @@ CURLcode Curl_waitfds_add_ps(struct curl_waitfds *cwfds,
       events |= CURL_WAIT_POLLIN;
     if(ps->actions[i] & CURL_POLL_OUT)
       events |= CURL_WAIT_POLLOUT;
-    if(events) {
-      if(cwfds_add_sock(cwfds, ps->sockets[i], events))
-        return CURLE_OUT_OF_MEMORY;
-    }
+    if(events)
+      need += cwfds_add_sock(cwfds, ps->sockets[i], events);
   }
-  return CURLE_OK;
+  return need;
 }
index f01acbdefc21d3dd935dca5863356e4b29f76e82..9d2d7bd21b95340b1756e4f184bac4aa40e66168 100644 (file)
@@ -140,8 +140,7 @@ void Curl_waitfds_init(struct curl_waitfds *cwfds,
                        struct curl_waitfd *static_wfds,
                        unsigned int static_count);
 
-CURLcode Curl_waitfds_add_ps(struct curl_waitfds *cwfds,
-                             struct easy_pollset *ps);
-
+unsigned int Curl_waitfds_add_ps(struct curl_waitfds *cwfds,
+                                 struct easy_pollset *ps);
 
 #endif /* HEADER_CURL_SELECT_H */
index 5b01cd8ce7afd27604ed1a7a7a035c3a353150dc..0d22083c913afae407465f75ff9d8d55dcd8c613 100644 (file)
  *  empty multi handle (expected zero descriptors),
  *  HTTP1 amd HTTP2 (no multiplexing) two transfers (expected two descriptors),
  *  HTTP2 with multiplexing (expected one descriptors)
+ *  Improper inputs to the API result in CURLM_BAD_FUNCTION_ARGUMENT.
+ *  Sending a empty ufds, and size = 0 will return the number of fds needed.
+ *  Sending a non-empty ufds, but smaller than the fds needed will result in a
+ *    CURLM_OUT_OF_MEMORY, and a number of fds that is >= to the number needed.
  *
  *  It is also expected that all transfers run by multi-handle should complete
  *  successfully.
@@ -158,11 +162,40 @@ static CURLcode test_run(char *URL, long option, unsigned int *max_fd_count)
   while(!mc) {
     /* get the count of file descriptors from the transfers */
     unsigned int fd_count = 0;
+    unsigned int fd_count_chk = 0;
 
     mc = curl_multi_perform(multi, &still_running);
     if(!still_running || mc != CURLM_OK)
       break;
 
+    /* verify improper inputs are treated correctly. */
+    mc = curl_multi_waitfds(multi, NULL, 0, NULL);
+
+    if(mc != CURLM_BAD_FUNCTION_ARGUMENT) {
+      fprintf(stderr, "curl_multi_waitfds() return code %d instead of "
+        "CURLM_BAD_FUNCTION_ARGUMENT.\n", mc);
+      res = TEST_ERR_FAILURE;
+      break;
+    }
+
+    mc = curl_multi_waitfds(multi, NULL, 1, NULL);
+
+    if(mc != CURLM_BAD_FUNCTION_ARGUMENT) {
+      fprintf(stderr, "curl_multi_waitfds() return code %d instead of "
+        "CURLM_BAD_FUNCTION_ARGUMENT.\n", mc);
+      res = TEST_ERR_FAILURE;
+      break;
+    }
+
+    mc = curl_multi_waitfds(multi, NULL, 1, &fd_count);
+
+    if(mc != CURLM_BAD_FUNCTION_ARGUMENT) {
+      fprintf(stderr, "curl_multi_waitfds() return code %d instead of "
+        "CURLM_BAD_FUNCTION_ARGUMENT.\n", mc);
+      res = TEST_ERR_FAILURE;
+      break;
+    }
+
     mc = curl_multi_waitfds(multi, ufds, 10, &fd_count);
 
     if(mc != CURLM_OK) {
@@ -174,8 +207,42 @@ static CURLcode test_run(char *URL, long option, unsigned int *max_fd_count)
     if(!fd_count)
       continue; /* no descriptors yet */
 
+    /* verify that sending nothing but the fd_count results in at least the
+     * same number of fds */
+    mc = curl_multi_waitfds(multi, NULL, 0, &fd_count_chk);
+
+    if(mc != CURLM_OK) {
+      fprintf(stderr, "curl_multi_waitfds() failed, code %d.\n", mc);
+      res = TEST_ERR_FAILURE;
+      break;
+    }
+
+    if(fd_count_chk < fd_count) {
+      fprintf(stderr, "curl_multi_waitfds() should return at least the number "
+        "of fds needed\n");
+      res = TEST_ERR_FAILURE;
+      break;
+    }
+
     /* checking case when we don't have enough space for waitfds */
-    mc = curl_multi_waitfds(multi, ufds1, fd_count - 1, NULL);
+    mc = curl_multi_waitfds(multi, ufds1, fd_count - 1, &fd_count_chk);
+
+    if(mc != CURLM_OUT_OF_MEMORY) {
+      fprintf(stderr, "curl_multi_waitfds() return code %d instead of "
+        "CURLM_OUT_OF_MEMORY.\n", mc);
+      res = TEST_ERR_FAILURE;
+      break;
+    }
+
+    if(fd_count_chk < fd_count) {
+      fprintf(stderr, "curl_multi_waitfds() sould return the amount of fds "
+        "needed if enough isn't passed in.\n");
+      res = TEST_ERR_FAILURE;
+      break;
+    }
+
+    /* sending ufds with zero size, is valid */
+    mc = curl_multi_waitfds(multi, ufds, 0, NULL);
 
     if(mc != CURLM_OUT_OF_MEMORY) {
       fprintf(stderr, "curl_multi_waitfds() return code %d instead of "
@@ -184,6 +251,22 @@ static CURLcode test_run(char *URL, long option, unsigned int *max_fd_count)
       break;
     }
 
+    mc = curl_multi_waitfds(multi, ufds, 0, &fd_count_chk);
+
+    if(mc != CURLM_OUT_OF_MEMORY) {
+      fprintf(stderr, "curl_multi_waitfds() return code %d instead of "
+        "CURLM_OUT_OF_MEMORY.\n", mc);
+      res = TEST_ERR_FAILURE;
+      break;
+    }
+
+    if(fd_count_chk < fd_count) {
+      fprintf(stderr, "curl_multi_waitfds() sould return the amount of fds "
+        "needed if enough isn't passed in.\n");
+      res = TEST_ERR_FAILURE;
+      break;
+    }
+
     if(fd_count > max_count)
       max_count = fd_count;