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
length*, the
sizeof is preferable there.
I find the change from char* to const char* acceptable, though. Or at least
|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 just
"old code". There are functions using the const keyword, so it is not a
case of
compatibility...
* GCC may actually be smart enough to resolve it at compilation time,
since 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
following warning:
> warning: initializer element is not a constant expression
|
