12:51 p.m.
When php disable call pass time by reference this breaks libvirt_stream_recv
and also bring more memory allocations from zend engine.
Vasiliy Tolstov (1):
fix libvirt_stream_send/libvirt_stream_recv
src/libvirt-php.c | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)
--
2.9.3
12:51 p.m.
Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
---
src/libvirt-php.c | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index e8384795d635..4e088e746281 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -178,9 +178,15 @@ ZEND_ARG_INFO(0, stats)
ZEND_ARG_INFO(0, flags)
ZEND_END_ARG_INFO()
-ZEND_BEGIN_ARG_INFO_EX(arginfo_libvirt_stream_send, 0, 0, 2)
+ZEND_BEGIN_ARG_INFO_EX(arginfo_libvirt_stream_send, 0, 1, 2)
ZEND_ARG_INFO(0, conn)
-ZEND_ARG_INFO(0, data)
+ZEND_ARG_INFO(1, data)
+ZEND_ARG_INFO(0, len)
+ZEND_END_ARG_INFO()
+
+ZEND_BEGIN_ARG_INFO_EX(arginfo_libvirt_stream_recv, 0, 1, 2)
+ZEND_ARG_INFO(0, conn)
+ZEND_ARG_INFO(1, data)
ZEND_ARG_INFO(0, len)
ZEND_END_ARG_INFO()
@@ -595,7 +601,7 @@ static zend_function_entry libvirt_functions[] = {
PHP_FE(libvirt_stream_abort, arginfo_libvirt_conn)
PHP_FE(libvirt_stream_finish, arginfo_libvirt_conn)
PHP_FE(libvirt_stream_send, arginfo_libvirt_stream_send)
- PHP_FE(libvirt_stream_recv, arginfo_libvirt_stream_send)
+ PHP_FE(libvirt_stream_recv, arginfo_libvirt_stream_recv)
/* Domain functions */
PHP_FE(libvirt_domain_new, arginfo_libvirt_domain_new)
PHP_FE(libvirt_domain_new_get_vnc, arginfo_libvirt_void)
@@ -4509,7 +4515,7 @@ PHP_FUNCTION(libvirt_stream_finish)
/*
* Function name: libvirt_stream_recv
* Since version: 0.5.0
- * Description: Function is used to close stream from libvirt conn
+ * Description: Function is used to recv from stream via libvirt conn
* Arguments: @res [resource]: libvirt stream resource from
libvirt_stream_create()
* @data [string]: buffer
* @len [int]: amout of data to recieve
@@ -4518,8 +4524,8 @@ PHP_FUNCTION(libvirt_stream_finish)
PHP_FUNCTION(libvirt_stream_recv)
{
zval *zstream, *zbuf;
- char *recv_buf;
php_libvirt_stream *stream = NULL;
+ char *recv_buf = NULL;
int retval = -1;
zend_long length = 0;
@@ -4534,31 +4540,24 @@ PHP_FUNCTION(libvirt_stream_recv)
retval = virStreamRecv(stream->stream, recv_buf, length);
if (retval < 0) {
- efree(recv_buf);
- zval_dtor(zbuf);
- ZVAL_NULL(zbuf);
+ zval_dtor(zbuf);
+ ZVAL_NULL(zbuf);
} else {
recv_buf[retval] = '\0';
-#if PHP_MAJOR_VERSION >= 7
- ZVAL_STRINGL(zbuf, recv_buf, retval);
- efree(recv_buf);
-#else
- ZVAL_STRINGL(zbuf, recv_buf, retval, 0);
-#endif
+ VIRT_ZVAL_STRINGL(zbuf, recv_buf, retval);
}
if (retval == -1) {
set_error("Cannot recv from stream" TSRMLS_CC);
- RETURN_LONG(retval);
}
-
+ efree(recv_buf);
RETURN_LONG(retval);
}
/*
* Function name: libvirt_stream_send
* Since version: 0.5.0
- * Description: Function is used to close stream from libvirt conn
+ * Description: Function is used to send to stream via libvirt conn
* Arguments: @res [resource]: libvirt stream resource from
libvirt_stream_create()
* @data [string]: buffer
* @length [int]: amout of data to send
@@ -4571,7 +4570,6 @@ PHP_FUNCTION(libvirt_stream_send)
int retval = -1;
zend_long length = 0;
char *cstr;
- //int cstrlen;
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rz|l", &zstream,
&zbuf, &length) == FAILURE)
RETURN_LONG(retval);
@@ -4580,12 +4578,10 @@ PHP_FUNCTION(libvirt_stream_send)
RETURN_LONG(retval);
cstr = Z_STRVAL_P(zbuf);
- //cstrlen = Z_STRLEN_P(zbuf);
retval = virStreamSend(stream->stream, cstr, length);
if (retval == -1) {
set_error("Cannot send to stream" TSRMLS_CC);
- RETURN_LONG(retval);
}
RETURN_LONG(retval);
--
2.9.3
8:06 a.m.
On 03/29/2017 07:51 PM, Vasiliy Tolstov wrote:
Signed-off-by: Vasiliy Tolstov <v.tolstov(a)selfip.ru>
---
src/libvirt-php.c | 36 ++++++++++++++++--------------------
1 file changed, 16 insertions(+), 20 deletions(-)
diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index e8384795d635..4e088e746281 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -178,9 +178,15 @@ ZEND_ARG_INFO(0, stats)
ZEND_ARG_INFO(0, flags)
ZEND_END_ARG_INFO()
-ZEND_BEGIN_ARG_INFO_EX(arginfo_libvirt_stream_send, 0, 0, 2)
+ZEND_BEGIN_ARG_INFO_EX(arginfo_libvirt_stream_send, 0, 1, 2)
ZEND_ARG_INFO(0, conn)
-ZEND_ARG_INFO(0, data)
+ZEND_ARG_INFO(1, data)
+ZEND_ARG_INFO(0, len)
+ZEND_END_ARG_INFO()
+
+ZEND_BEGIN_ARG_INFO_EX(arginfo_libvirt_stream_recv, 0, 1, 2)
+ZEND_ARG_INFO(0, conn)
+ZEND_ARG_INFO(1, data)
ZEND_ARG_INFO(0, len)
ZEND_END_ARG_INFO()
@@ -595,7 +601,7 @@ static zend_function_entry libvirt_functions[] = {
PHP_FE(libvirt_stream_abort, arginfo_libvirt_conn)
PHP_FE(libvirt_stream_finish, arginfo_libvirt_conn)
PHP_FE(libvirt_stream_send, arginfo_libvirt_stream_send)
- PHP_FE(libvirt_stream_recv, arginfo_libvirt_stream_send)
+ PHP_FE(libvirt_stream_recv, arginfo_libvirt_stream_recv)
/* Domain functions */
PHP_FE(libvirt_domain_new, arginfo_libvirt_domain_new)
PHP_FE(libvirt_domain_new_get_vnc, arginfo_libvirt_void)
@@ -4509,7 +4515,7 @@ PHP_FUNCTION(libvirt_stream_finish)
/*
* Function name: libvirt_stream_recv
* Since version: 0.5.0
- * Description: Function is used to close stream from libvirt conn
+ * Description: Function is used to recv from stream via libvirt conn
* Arguments: @res [resource]: libvirt stream resource from
libvirt_stream_create()
* @data [string]: buffer
* @len [int]: amout of data to recieve
@@ -4518,8 +4524,8 @@ PHP_FUNCTION(libvirt_stream_finish)
PHP_FUNCTION(libvirt_stream_recv)
{
zval *zstream, *zbuf;
- char *recv_buf;
php_libvirt_stream *stream = NULL;
+ char *recv_buf = NULL;
int retval = -1;
zend_long length = 0;
@@ -4534,31 +4540,24 @@ PHP_FUNCTION(libvirt_stream_recv)
There's memset() just above this line. It is completely useless -
because the recv_buf will be overwritten straight away. I'm appending
its removal to this patch.
retval = virStreamRecv(stream->stream, recv_buf, length);
if (retval < 0) {
- efree(recv_buf);
- zval_dtor(zbuf);
- ZVAL_NULL(zbuf);
+ zval_dtor(zbuf);
+ ZVAL_NULL(zbuf);
This space change does not look right.
} else {
recv_buf[retval] = '\0';
Frankly, I've no idea whether this should be here. I feel like no
because VIRT_ZVAL_STRINGL should not look into the recv_buf at any
position greater than retval. That is why we give it the length of the
string after all. Can you please check whether your case still works
after this line is removed?
-#if PHP_MAJOR_VERSION >= 7
- ZVAL_STRINGL(zbuf, recv_buf, retval);
- efree(recv_buf);
-#else
- ZVAL_STRINGL(zbuf, recv_buf, retval, 0);
-#endif
+ VIRT_ZVAL_STRINGL(zbuf, recv_buf, retval);
}
if (retval == -1) {
set_error("Cannot recv from stream" TSRMLS_CC);
- RETURN_LONG(retval);
}
-
+ efree(recv_buf);
RETURN_LONG(retval);
}
/*
* Function name: libvirt_stream_send
* Since version: 0.5.0
- * Description: Function is used to close stream from libvirt conn
+ * Description: Function is used to send to stream via libvirt conn
* Arguments: @res [resource]: libvirt stream resource from
libvirt_stream_create()
* @data [string]: buffer
* @length [int]: amout of data to send
@@ -4571,7 +4570,6 @@ PHP_FUNCTION(libvirt_stream_send)
int retval = -1;
zend_long length = 0;
char *cstr;
- //int cstrlen;
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "rz|l", &zstream,
&zbuf, &length) == FAILURE)
RETURN_LONG(retval);
@@ -4580,12 +4578,10 @@ PHP_FUNCTION(libvirt_stream_send)
RETURN_LONG(retval);
cstr = Z_STRVAL_P(zbuf);
- //cstrlen = Z_STRLEN_P(zbuf);
retval = virStreamSend(stream->stream, cstr, length);
if (retval == -1) {
set_error("Cannot send to stream" TSRMLS_CC);
- RETURN_LONG(retval);
}
I'm dropping these curly braces around one line body.
RETURN_LONG(retval);
ACKed and pushed.
Michal
8:18 a.m.
2017-03-30 16:06 GMT+03:00 Michal Privoznik <mprivozn(a)redhat.com>:
Frankly, I've no idea whether this should be here. I feel like no
because
VIRT_ZVAL_STRINGL should not look into the recv_buf at any position greater
than retval. That is why we give it the length of the string after all. Can
you please check whether your case still works after this line is removed?
I'm try raw images and recv_buf[retval] = '\0'; does not bring any
problems (md5sum on source and dest equal with and without this).
But can you look into send function in this function we convert zval
to C string. As i understand leading zero dropped in this case ?
--
Vasiliy Tolstov,
e-mail: v.tolstov(a)selfip.ru
8:32 a.m.
On 03/30/2017 03:18 PM, Vasiliy Tolstov wrote:
2017-03-30 16:06 GMT+03:00 Michal Privoznik
<mprivozn(a)redhat.com>:
> Frankly, I've no idea whether this should be here. I feel like no because
> VIRT_ZVAL_STRINGL should not look into the recv_buf at any position greater
> than retval. That is why we give it the length of the string after all. Can
> you please check whether your case still works after this line is removed?
I'm try raw images and recv_buf[retval] = '\0'; does not bring any
problems (md5sum on source and dest equal with and without this).
But can you look into send function in this function we convert zval
to C string. As i understand leading zero dropped in this case ?
I guess so. So if you want to drop the line feel free to propose the
patch. If you don't feel like touching the code - that is okay too.
Michal
2824
days inactive
2825
days old
4 comments
2 participants
participants (2)
-
Michal Privoznik -
Vasiliy Tolstov