-
Notifications
You must be signed in to change notification settings - Fork 7.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
curl: Add curl_multi_get_handles()
#16363
Conversation
f0c466e
to
a6bbd63
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea seems sensible to me.
|
||
array_init(return_value); | ||
zend_llist_position pos; | ||
zval *pz_ch; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tab character seems unintentional here?
zval *pz_ch; | |
zval *pz_ch; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've stolen the logic as-is from curl_multi_exec
and did not adjust the code style to keep the file internally consistent. This also applies to the other remark.
There would be more to clean up here, for example the removal of the useless (zval *)
cast, because a void*
casts implicitly to anything.
Perhaps it would even make sense to migrate from zend_llist
to HashTable
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, let's move this to a follow-up PR then.
zend_llist_position pos; | ||
zval *pz_ch; | ||
|
||
for (pz_ch = (zval *)zend_llist_get_first_ex(&mh->easyh, &pos); pz_ch; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could merge the declaration within the for loop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm :-) Thanks @TimWolla
see https://curl.se/libcurl/c/curl_multi_get_handles.html