Re: [PATCH] [libvirt-php] fix libvirt_stream_send/libvirt_stream_recv

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/29/2017 07:51 PM, Vasiliy Tolstov wrote:
Signed-off-by: Vasiliy Tolstov <v.tolstov@xxxxxxxxx>
---
 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux