Cast trap with INI entries compiling C++ extension View as Markdown

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