Hi everyone,

I've been developing a PHP extension for internal needs.
We're using C++, by using PHP_REQUIRE_CXX() in config.m4.
I'm using debian sid 64bits, with the package php5-dev-5.3.8-2
(against which the patch below has been created).

When declaring an INI entry, like this:
    PHP_INI_BEGIN()
        STD_PHP_INI_ENTRY("extensionname.variable1", NULL,
PHP_INI_ALL, OnUpdateString, globalsVariable1,
zend_extensionname_globals, extensionname_globals)
        STD_PHP_INI_ENTRY("extensionname.variable2", NULL,
PHP_INI_ALL, OnUpdateString, globalsVariable2,
zend_extensionname_globals, extensionname_globals)
    PHP_INI_END()
We get the following warning:
    warning: deprecated conversion from string constant to 'char*'
[-Wwrite-strings]
I have a bunch of those, one per INI entry, and prefer to get rid of them.

As a usual fix, I changed the line to use char* C-style cast as follows:
    STD_PHP_INI_ENTRY((char*)"extensionname.variable1", NULL,
PHP_INI_ALL, OnUpdateString, globalsVariable1,
zend_extensionname_globals, extensionname_globals)
    STD_PHP_INI_ENTRY((char*)"extensionname.variable2", NULL,
PHP_INI_ALL, OnUpdateString, globalsVariable2,
zend_extensionname_globals, extensionname_globals)
However, this does not work, I can see through phpinfo(), listing the
extension's INI entries, that only "extensi" is listed.
I'm aware that const char* should be used instead, but even using
strdup("extensionname.variableN") does not work!

It took me a while to find the problem.
STD_PHP_INI_ENTRY resolves to STD_ZEND_INI_ENTRY which resolves to
ZEND_INI_ENTRY2, then ZEND_INI_ENTRY2_EX then in turn
ZEND_INI_ENTRY3_EX which finally uses the following with the first
macro parameter "name":
    [...], name, sizeof(name), [...]
Yep, the problem comes from the sizeof(), which works on a string
constant seemingly interpreted as a (const?) char[] in C and C++,
returning sizeof(void*) instead of the length of the string (+ the
trailing \0), when using the classic char* cast.
Fine as this is probably not the right cast (it is really a very
common mistake), but just before we are setting a field of type char*,
which is the one that raises the warning.

My point is that there is a problem if it is that easy to trigger a
bug with some "natural reflex" to get rid of a warning.
I suggest some fixes:
* Use strlen() instead of sizeof().
* Use in-macro cast to char[].
* Use const when the string value won't be modified (I'm not talking
about the pointer, but its content).
In fact, I propose the following changes so that no user (extension
writer) code has to change:
--- zend_ini.h  2011-09-29 12:24:39.012882527 +0200
+++ zend_ini.h  2011-09-29 12:24:32.552577965 +0200
@@ -65,14 +65,14 @@
 struct _zend_ini_entry {
        int module_number;
        int modifiable;
-       char *name;
+       const char *name;
        uint name_length;
        ZEND_INI_MH((*on_modify));
        void *mh_arg1;
        void *mh_arg2;
        void *mh_arg3;

-       char *value;
+       const char *value;
        uint value_length;

        char *orig_value;
@@ -117,7 +117,7 @@
 #define ZEND_INI_END()         { 0, 0, NULL, 0, NULL, NULL, NULL,
NULL, NULL, 0, NULL, 0, 0, 0, NULL } };

 #define ZEND_INI_ENTRY3_EX(name, default_value, modifiable,
on_modify, arg1, arg2, arg3, displayer) \
-       { 0, modifiable, name, sizeof(name), on_modify, arg1, arg2,
arg3, default_value, sizeof(default_value)-1, NULL, 0, 0, 0, displayer
},
+       { 0, modifiable, name, strlen(name)+1, on_modify, arg1, arg2,
arg3, default_value, default_value != NULL ? strlen(default_value) :
sizeof(NULL), NULL, 0, 0, 0, displayer },

 #define ZEND_INI_ENTRY3(name, default_value, modifiable, on_modify,
arg1, arg2, arg3) \
        ZEND_INI_ENTRY3_EX(name, default_value, modifiable, on_modify,
arg1, arg2, arg3, NULL)

We use const char* fields not to trigger the C++ deprecation warning,
and we use strlen() to get the size of the string (which is the only
normal way anyway), but test for a non NULL value (useless for "name"
I guess). By the way, I still return sizeof(NULL) for compatibility,
but 0 should probably be a better value.

I only tested that change with building my C++ PHP extension, whole
PHP recompilation.


Best,
---
Olivier Favre
Software engineer
Yakaz
http://www.yakaz.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to