Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:55664
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain yakaz.com designates 209.85.216.177 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <4E848B31.2030501@gmail.com>
References: <CAD83JAVvBQ-zjR2Fnmf1MQwf_Rh+OQjqWsjUeNqB4WTjxf-TOw@mail.gmail.com>
 <4E848B31.2030501@gmail.com>
Date: Thu, 29 Sep 2011 17:42:19 +0200
Message-ID: <CAD83JAU_TOf2YimY_2C1DarFFDkfLFWoZrUZaMixOtBDa-8yUg@mail.gmail.com>
To: =?ISO-8859-1?Q?=C1ngel_Gonz=E1lez?= <keisial@gmail.com>
Cc: internals@lists.php.net
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
Subject: Re: [PHP-DEV] Cast trap with INI entries compiling C++ extension
From: olivier@yakaz.com (Olivier Favre)

I checked with a tiny test program, you're right about GCC complaining.
The right fix is to make the field const (I don't know about const keyword)=
.
G++ won't give warnings, no error would be triggered by a broken fix.

By the way, const char* and char const* are the same, you probably
meant char * const.
But that wouldn't prevent by a compilation error changing the
referenced chars, and may lead to segfaults if that happens.
As no segfault happens (fortunately!) I we can infer that the string
constants aren't modified (hopefully!).
Therefore we should use const char*, or even const char* const.
(Finally maybe keywords are more suited for the task, depending on how
they work ;-)

Another thing, g++ raises another warning with the last field that
STANDARD_MODULE_PROPERTIES_EX sets in the zend_module_entry structure
(declared in Zend/zend_modules.h:101).
Guess what, it is a char* too. Other fields of the structure are set
to const char* though.

Conclusion: I thing const char * should be used, for consistency.

By compiling my extension, I didn't see any other warnings.


Thanks,
---
Olivier Favre
Software engineer
Yakaz
http://www.yakaz.com

2011/9/29 =C1ngel Gonz=E1lez <keisial@gmail.com>:
> On 29/09/11 14:14, Olivier Favre wrote:
>
> 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).
>
> (...)
>
> 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:
>
> (...)
>
> 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,
>
> Using strlen() there forces a runtime call to figure out the string lengt=
h*,
> the
> sizeof is preferable there.
> I find the change from char* to const char* acceptable, though. Or at lea=
st
> char const*, I'm not sure if those values are changed at runtime.
> I have found in the past some places where a const keyword would be
> preferable,
> but was't used. I don't know if there's a rationale for that or is it jus=
t
> "old code". There are functions using the const keyword, so it is not a c=
ase
> of
> compatibility...
>
>
> * GCC may actually be smart enough to resolve it at compilation time, sin=
ce
> it
> implements strlen() as a builtin. But you obviously can't count on that. =
I'm
> not
> even sure if that's legal C (or from which version). g++ doesn't complain=
,
> but with
> your patch of adding strlen(), I think gcc gives (on C files) the followi=
ng
> warning:
>> warning: initializer element is not a constant expression
>
>
>