Re: [libvirt] [libvirt-php PATCH 09/35] adapt add_assoc_string_ex

2016-04-12 Thread Neal Gompa 
On Tue, Apr 12, 2016 at 11:13 AM, Michal Privoznik  wrote:
>
> I'm afraid, this will not fly. While preprocessing this code, my
> compiler tries to expand RETURN_STRING() macro first (with VIRT_COPY_OPT
> still not expanded). Therefore it finds an argument missing and produces
> an compilation error.
> What we can do here is create a wrapper over RETURN_STRING, e.g.
> VIR_RETURN_STRING() that will behave differently for PHP7 and the rest.
>
> Unfortunately, this is where I have to stop the review as it's getting
> hairy and I gotta run. Sorry.
>
> Michal

Unfortunately, I cannot reproduce your problem. On my CentOS 7.2
system, I built, installed, and tested using gcc-4.8.5-4.el7 with
libvirt-1.2.17-13.el7_2.4 and its associated devel package. I've
compiled it as a PHP 5.4 module (using the default PHP), as a PHP 5.6
module (using Remi's SCL for PHP 5.6), and as a PHP 7.0 module (using
Remi's SCL for PHP 7.0). It worked properly with both build systems,
and all 17 tests passed with the `make test` run on the newer build
system.

For reference on my CentOS 7.2 system, I'm using the following:
* gcc-4.8.5-4.el7
* libvirt-daemon-1.2.17-13.el7_2.4
* libvirt-devel-1.2.17-13.el7_2.4
* xz-devel-5.1.2-12alpha.el7
* libxml2-devel-2.9.1-6.el7_2.2
* libxslt-1.1.28-5.el7
* xhtml1-dtds-1.0-20020801.11.el7

Here are the PHP packages I have installed:
* php-cli-5.4.16-36.el7_1
* php-common-5.4.16-36.el7_1
* php-devel-5.4.16-36.el7_1
* php56-runtime-2.1-5.el7.remi
* php56-php-cli-5.6.20-1.el7.remi
* php56-php-pecl-jsonc-1.3.9-1.el7.remi
* php56-php-pecl-zip-1.13.2-1.el7.remi
* php56-php-pecl-jsonc-devel-1.3.9-1.el7.remi
* php56-php-common-5.6.20-1.el7.remi
* php56-php-devel-5.6.20-1.el7.remi
* php70-runtime-1.0-4.el7.remi
* php70-php-json-7.0.5-1.el7.remi
* php70-php-cli-7.0.5-1.el7.remi
* php70-php-common-7.0.5-1.el7.remi
* php70-php-devel-7.0.5-1.el7.remi


For kicks, I tested on Fedora 23 with the latest system libvirt and
php 5.6 packages available, and everything seems to work as expected.

Here are the packages on my Fedora system:
* gcc-5.3.1-2.fc23
* libvirt-daemon-1.2.18.2-3.fc23
* libvirt-devel-1.2.18.2-3.fc23
* xz-devel-5.2.1-3.fc23
* libxml2-devel-2.9.3-2.fc23
* libxslt-1.1.28-11.fc23
* xhtml1-dtds-1.0-20020801.13.fc23

Here are the PHP packages on my Fedora system:
* php-cli-5.6.20-1.fc23
* php-common-5.6.20-1.fc23
* php-pecl-jsonc-1.3.9-1.fc23
* php-pecl-jsonc-devel-1.3.9-1.fc23
* php-devel-5.6.20-1.fc23

Obviously there's some kind of mismatch in my environment and yours
that is causing problems. Could you please tell me what your
environment is and what you're using so that I can see if I can
reproduce the issue?

-- 
真実はいつも一つ！/ Always, there's only one truth!

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

Re: [libvirt] [libvirt-php PATCH 09/35] adapt add_assoc_string_ex

2016-04-12 Thread Michal Privoznik 
On 09.04.2016 00:08, Neal Gompa wrote:
> From: Remi Collet 
> 
> ---
>  src/libvirt-php.c | 299 
> ++
>  1 file changed, 99 insertions(+), 200 deletions(-)
> 
> diff --git a/src/libvirt-php.c b/src/libvirt-php.c
> index d37fd6f..0459bb9 100644
> --- a/src/libvirt-php.c
> +++ b/src/libvirt-php.c
> @@ -58,6 +58,7 @@ const char *features_binaries[] = { NULL };
>  #if PHP_MAJOR_VERSION >= 7
>  typedef size_t strsize_t;
>  
> +#define VIRT_COPY_OPT
>  #define VIRT_FETCH_RESOURCE(_state, _type, _zval, _name, _le) \
>   if ((_state = (_type)zend_fetch_resource(Z_RES_P(*_zval), _name, _le)) 
> == NULL) { \
>   RETURN_FALSE; \
> @@ -68,6 +69,8 @@ typedef int strsize_t;
>  typedef long zend_long;
>  typedef unsigned long zend_ulong;
>  
> +#define VIRT_COPY_OPT ,1
> +
>  #define VIRT_FETCH_RESOURCE(_state, _type, _zval, _name, _le) \
>   ZEND_FETCH_RESOURCE(_state, _type, _zval, -1, _name, _le);
>  
> @@ -1995,7 +1998,7 @@ if ((snapshot==NULL) || (snapshot->snapshot==NULL)) 
> RETURN_FALSE;\
>  #define LONGLONG_ASSOC(out,key,in) \
>  if (LIBVIRT_G(longlong_to_string_ini)) { \
>  snprintf(tmpnumber,63,"%llu",in); \
> -add_assoc_string_ex(out,key,strlen(key)+1,tmpnumber,1); \
> +add_assoc_string_ex(out,key,strlen(key)+1,tmpnumber VIRT_COPY_OPT); \
>  } \
>  else \
>  { \
> @@ -2059,11 +2062,7 @@ static int libvirt_virConnectCredType[] = {
>  PHP_FUNCTION(libvirt_get_last_error)
>  {
>  if (LIBVIRT_G (last_error) == NULL) RETURN_NULL();
> -#if PHP_MAJOR_VERSION >= 7
> -RETURN_STRING(LIBVIRT_G (last_error));
> -#else
> -RETURN_STRING(LIBVIRT_G (last_error),1);
> -#endif
> +RETURN_STRING(LIBVIRT_G (last_error) VIRT_COPY_OPT);

I'm afraid, this will not fly. While preprocessing this code, my
compiler tries to expand RETURN_STRING() macro first (with VIRT_COPY_OPT
still not expanded). Therefore it finds an argument missing and produces
an compilation error.
What we can do here is create a wrapper over RETURN_STRING, e.g.
VIR_RETURN_STRING() that will behave differently for PHP7 and the rest.

Unfortunately, this is where I have to stop the review as it's getting
hairy and I gotta run. Sorry.

Michal

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

[libvirt] [libvirt-php PATCH 09/35] adapt add_assoc_string_ex

2016-04-08 Thread Neal Gompa 
From: Remi Collet 

---
 src/libvirt-php.c | 299 ++
 1 file changed, 99 insertions(+), 200 deletions(-)

diff --git a/src/libvirt-php.c b/src/libvirt-php.c
index d37fd6f..0459bb9 100644
--- a/src/libvirt-php.c
+++ b/src/libvirt-php.c
@@ -58,6 +58,7 @@ const char *features_binaries[] = { NULL };
 #if PHP_MAJOR_VERSION >= 7
 typedef size_t strsize_t;
 
+#define VIRT_COPY_OPT
 #define VIRT_FETCH_RESOURCE(_state, _type, _zval, _name, _le) \
if ((_state = (_type)zend_fetch_resource(Z_RES_P(*_zval), _name, _le)) 
== NULL) { \
RETURN_FALSE; \
@@ -68,6 +69,8 @@ typedef int strsize_t;
 typedef long zend_long;
 typedef unsigned long zend_ulong;
 
+#define VIRT_COPY_OPT ,1
+
 #define VIRT_FETCH_RESOURCE(_state, _type, _zval, _name, _le) \
ZEND_FETCH_RESOURCE(_state, _type, _zval, -1, _name, _le);
 
@@ -1995,7 +1998,7 @@ if ((snapshot==NULL) || (snapshot->snapshot==NULL)) 
RETURN_FALSE;\
 #define LONGLONG_ASSOC(out,key,in) \
 if (LIBVIRT_G(longlong_to_string_ini)) { \
 snprintf(tmpnumber,63,"%llu",in); \
-add_assoc_string_ex(out,key,strlen(key)+1,tmpnumber,1); \
+add_assoc_string_ex(out,key,strlen(key)+1,tmpnumber VIRT_COPY_OPT); \
 } \
 else \
 { \
@@ -2059,11 +2062,7 @@ static int libvirt_virConnectCredType[] = {
 PHP_FUNCTION(libvirt_get_last_error)
 {
 if (LIBVIRT_G (last_error) == NULL) RETURN_NULL();
-#if PHP_MAJOR_VERSION >= 7
-RETURN_STRING(LIBVIRT_G (last_error));
-#else
-RETURN_STRING(LIBVIRT_G (last_error),1);
-#endif
+RETURN_STRING(LIBVIRT_G (last_error) VIRT_COPY_OPT);
 }
 
 /*
@@ -2223,7 +,7 @@ PHP_FUNCTION(libvirt_node_get_info)
 if (retval==-1) RETURN_FALSE;
 
 array_init(return_value);
-add_assoc_string_ex(return_value, "model", 6, info.model, 1);
+add_assoc_string_ex(return_value, "model", 6, info.model VIRT_COPY_OPT);
 add_assoc_long(return_value, "memory", (long)info.memory);
 add_assoc_long(return_value, "cpus", (long)info.cpus);
 add_assoc_long(return_value, "nodes", (long)info.nodes);
@@ -2318,9 +2317,9 @@ PHP_FUNCTION(libvirt_node_get_cpu_stats)
 add_assoc_long(return_value, "cpu", cpunr);
 else
 if (cpuNum == VIR_NODE_CPU_STATS_ALL_CPUS)
-add_assoc_string_ex(return_value, "cpu", 4, "all", 1);
+add_assoc_string_ex(return_value, "cpu", 4, "all" VIRT_COPY_OPT);
 else
-add_assoc_string_ex(return_value, "cpu", 4, "unknown", 1);
+add_assoc_string_ex(return_value, "cpu", 4, "unknown" 
VIRT_COPY_OPT);
 
 free(params);
 params = NULL;
@@ -2532,21 +2531,21 @@ PHP_FUNCTION(libvirt_connect_get_machine_types)
 char tmp3[2048] = { 0 };
 
 snprintf(key, sizeof(key), "%d", k);
-//add_assoc_string_ex(arr2, key, strlen(key) + 1, 
ret3[k], 1);
+//add_assoc_string_ex(arr2, key, strlen(key) + 1, 
ret3[k] VIRT_COPY_OPT);
 
 snprintf(tmp3, sizeof(tmp3), 
"//capabilities/guest/arch[@name=\"%s\"]/machine[text()=\"%s\"]/@maxCpus",
  ret[i], ret3[k]);
 
 numTmp = get_string_from_xpath(caps, tmp3, NULL, 
NULL);
 if (numTmp == NULL)
-add_assoc_string_ex(arr2, key, strlen(key) + 
1, ret3[k], 1);
+add_assoc_string_ex(arr2, key, strlen(key) + 
1, ret3[k] VIRT_COPY_OPT);
 else {
 zval *arr4;
 ALLOC_INIT_ZVAL(arr4);
 array_init(arr4);
 
-add_assoc_string_ex(arr4, "name", 5, ret3[k], 
1);
-add_assoc_string_ex(arr4, "maxCpus", 9, 
numTmp, 1);
+add_assoc_string_ex(arr4, "name", 5, ret3[k] 
VIRT_COPY_OPT);
+add_assoc_string_ex(arr4, "maxCpus", 9, numTmp 
VIRT_COPY_OPT);
 
 add_assoc_zval_ex(arr2, key, strlen(key) + 1, 
arr4);
 free(numTmp);
@@ -2574,14 +2573,14 @@ PHP_FUNCTION(libvirt_connect_get_machine_types)
 
 numTmp = get_string_from_xpath(caps, tmp3, NULL, 
NULL);
 if (numTmp == NULL)
-add_assoc_string_ex(arr3, key, strlen(key) + 
1, ret3[k], 1);
+add_assoc_string_ex(arr3, key, strlen(key) + 
1, ret3[k] VIRT_COPY_OPT);
 else {
 zval *arr4;
 ALLOC_INIT_ZVAL(arr4);
 array_init(arr4);
 
-add_assoc_string_ex(arr4, "name", 5, ret3[k], 
1);
-