On 18-Dec-23 10:40, Daniel Stenberg via curl-library wrote:
On Thu, 14 Dec 2023, Dmitry Karpov via curl-library wrote:

Probably, it would make sense to also return the count of descriptors, like:

CURLMcode curl_multi_pollfds(struct curl_waitfd *ufds, unsigned int size, unsigned int* fd_count);

The fd_count will tell the client code how many descriptors the multi-handle actually has.

How about this tweak:

 CURLMcode
 curl_multi_pollfds(struct Curl_multi *multi,
                    struct curl_waitfd **ufds,
                    unsigned int *fd_count);

It returns a pointer to an array of descriptors and fd_count is the number of entries in that array. The array is allocated by libcurl and is stored associated with the multi handle so it is freed with the handle. Calling this function several times will then just overwrite the same array so it does not have to get reallocated, unless it needs to grow, during a program's lifetime.

It seems less complicated than having the caller pass in an array of a size it can't know how big it needs to be. The downside is of course that the data in the array will not survive calling the function again.

Is there a thread safety issue with this?  E.g.  one thread calls, buffer changes due to allocating more FDs in second thread, first thread now has cached pointer to freed memory (use after free).

In any case, the traditional way for the user to get the buffer size required is to return the needed size with a non-fatal error code.  You could use the proposed signature but have the caller initialize *ufds & count.  If *ufds is NULL or count is less than required, just return the count (required size) in *fd_count & and error code (CURL_BUFFER_TOO_SMALL).  Caller (re-)alloc's a buffer of at least that size (sizeof struct *curl_waitfd)* *fd_count and retries.  This leaves the caller responsible for both allocating and deallocating the buffer.   It's not data used by libcurl, so making libcurl take responsibility for it seems odd.

This also raises the question of how the caller knows that it's necessary to get an updated list of active fds.  Presumably, libcurl can allocate/free fds for its internal use.  There may not be a simple mapping between a caller's actions on handles and the fds that need to be polled.  You will get an EBADF from poll if an fd is closed, or confused if it's been recycled.

Another approach would be for libcurl to register a callback triggered when an fd is opened/closed, and provide the user with a curl_waitfd & open/close flag.  (Model on CURLOPT_{OPEN,CLOSE}SOCKETFUNCTION) The user has more bookkeeping to do, but libcurl doesn't have to worry about freeing this user buffer.  And the user knows exactly what fds are active.


Timothe Litt
ACM Distinguished Engineer
--------------------------
This communication may not represent the ACM or my employer's views,
if any, on the matters discussed.

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature

-- 
Unsubscribe: https://lists.haxx.se/mailman/listinfo/curl-library
Etiquette:   https://curl.se/mail/etiquette.html

Reply via email to