-
Notifications
You must be signed in to change notification settings - Fork 504
Added static method Socket::createListen() #844
base: master
Are you sure you want to change the base?
Conversation
70dfe10
to
f89a332
Compare
#else | ||
if ((hp = php_network_gethostbyname("localhost")) == NULL) { | ||
#endif | ||
zval_ptr_dtor(return_value); |
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.
@tpunt is zval_ptr_dtor
used correctly at this point and below?
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.
This looks fine to me. Technically, zval_dtor
could be used, since we know that the newly created socket object will not contain a cycle (so there's no need to place it into the GC root buffer). But it doesn't really matter.
pthreads_object_t *created; | ||
pthreads_socket_t *sock; | ||
|
||
object_init_ex(return_value, pthreads_socket_entry); |
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.
This can return FAILURE
. It's not immediately clear to me if any such paths exists where this can fail (from a quick look through the source code), but it may be worthwhile checking its return value for failure just in case I've missed some psychotic edge case.
@@ -164,6 +166,7 @@ zend_function_entry pthreads_socket_methods[] = { | |||
PHP_ME(Socket, close, Socket_void, ZEND_ACC_PUBLIC) | |||
PHP_ME(Socket, getLastError, Socket_getLastError, ZEND_ACC_PUBLIC) | |||
PHP_ME(Socket, clearError, Socket_void, ZEND_ACC_PUBLIC) | |||
PHP_ME(Socket, createListen, Socket_void, ZEND_ACC_PUBLIC|ZEND_ACC_STATIC) |
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.
This doesn't look right. A separate arginfo needs to be defined for this.
createListen()
wrapscreate()
,bind()
andlisten()
in one call