Re: [PHP-DEV] Cast trap with INI entries compiling C++ extension
I finally managed to submit the patch... I used iceweasel 6.0.2
instead of Google Chrome 14.0.835.186... or maybe the captcha was just
in a better mood!
The link to the bug report: https://bugs.php.net/bug.php?id=55835
Yours,
--
Olivier Favre
Software engineer
Yakaz
www.yakaz.com
2011/9/30 Olivier Favre oliv...@yakaz.com
I keep on having the following error:
ERROR:
⋅ Incorrect Captcha
while trying to file a bug on the following page:
https://bugs.php.net/report.php
(Debian sid, Google Chrome 14.0.835.186)
I tried flushing my cookies.
There are two opened bug reports about that:
- https://bugs.php.net/bug.php?id=54380
- https://bugs.php.net/bug.php?id=53255
And I would have liked to file another... if only I could!
Anyway, if someone else is luckier, here what I'd have liked to file:
PHP version: 5.3.8
Package affected: Compile issues/Compilation warning
Bug type: Feature/Change request
OS: All (seen under Linux)
Summary: char* field should be const char* to avoid C++ warning
Description:
http://news.php.net/php.internals/55662
I'm writing a C++ extension to PHP.
When declaring a INI entry I get the following warning, multiple times:
warning: deprecated conversion from string constant to 'char*'
[-Wwrite-strings]
This only arises when compiling with a C++ compiler.
The right and easy fix seems to set some fields to const char *.
Some may even be set to const char * const (but this alternative seems
to be used
nowhere).
The proposed patch is against the php5-dev-5.3.8-2 package of debian sid:
PHP 5.3.8-2 with Suhosin-Patch (cli) (built: Sep 12 2011 07:28:26)
- - -
Test script:
Write a C++ extension:
config.m4 should contain PHP_REQUIRE_CXX().
Declare your module:
zend_module_entry quezako_module_entry = {
STANDARD_MODULE_HEADER,
YourExtensionName, // (1 warning here)
[...],
0.42, // (1 warning here)
[...],
STANDARD_MODULE_PROPERTIES_EX
};
Declare an INI entry:
PHP_INI_BEGIN()
STD_PHP_INI_ENTRY(
extensionName.variable, // (1 warning here)
default value, // (1 warning here)
[...]
)
PHP_INI_END()
- - -
Patch name: field_constness_cpp_compilation_warning_fix.patch
Patch file: (see attached file)
Expected result:
No compilation warning.
- - -
Actual result:
Multiple of the following warning:
warning: deprecated conversion from string constant to 'char*'
[-Wwrite-strings]
Using the very common fix of prepending (char*) to the string constant is
especially harmful here, because of ZEND_INI_ENTRY3_EX using sizeof() on in:
it
returns 4/8 (32/64bits platform).
Using a cast to char[] solves the problem, but the above fix is a very very
common mistake.
If the target fields were const char*, no compilation warning would be rose.
- - -
Solve the problem 17 + 23 = ?: 40 (I even checked the answer using a
calculator)
Submit: Send bug report (I'm going mad, really...)
--
Olivier Favre
Software engineer
Yakaz
www.yakaz.com
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Cast trap with INI entries compiling C++ extension
there was a problem with bugs.php.net, so the captcha error was on our end. thanks for the report On Mon, Oct 3, 2011 at 10:01 AM, Olivier Favre oliv...@yakaz.com wrote: I finally managed to submit the patch... I used iceweasel 6.0.2 instead of Google Chrome 14.0.835.186... or maybe the captcha was just in a better mood! -- Ferenc Kovács @Tyr43l - http://tyrael.hu -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Cast trap with INI entries compiling C++ extension
I keep on having the following error:
ERROR:
⋅ Incorrect Captcha
while trying to file a bug on the following page:
https://bugs.php.net/report.php
(Debian sid, Google Chrome 14.0.835.186)
I tried flushing my cookies.
There are two opened bug reports about that:
- https://bugs.php.net/bug.php?id=54380
- https://bugs.php.net/bug.php?id=53255
And I would have liked to file another... if only I could!
Anyway, if someone else is luckier, here what I'd have liked to file:
PHP version: 5.3.8
Package affected: Compile issues/Compilation warning
Bug type: Feature/Change request
OS: All (seen under Linux)
Summary: char* field should be const char* to avoid C++ warning
Description:
http://news.php.net/php.internals/55662
I'm writing a C++ extension to PHP.
When declaring a INI entry I get the following warning, multiple times:
warning: deprecated conversion from string constant to 'char*' [-Wwrite-strings]
This only arises when compiling with a C++ compiler.
The right and easy fix seems to set some fields to const char *.
Some may even be set to const char * const (but this alternative seems
to be used
nowhere).
The proposed patch is against the php5-dev-5.3.8-2 package of debian sid:
PHP 5.3.8-2 with Suhosin-Patch (cli) (built: Sep 12 2011 07:28:26)
- - -
Test script:
Write a C++ extension:
config.m4 should contain PHP_REQUIRE_CXX().
Declare your module:
zend_module_entry quezako_module_entry = {
STANDARD_MODULE_HEADER,
YourExtensionName, // (1 warning here)
[...],
0.42, // (1 warning here)
[...],
STANDARD_MODULE_PROPERTIES_EX
};
Declare an INI entry:
PHP_INI_BEGIN()
STD_PHP_INI_ENTRY(
extensionName.variable, // (1 warning here)
default value, // (1 warning here)
[...]
)
PHP_INI_END()
- - -
Patch name: field_constness_cpp_compilation_warning_fix.patch
Patch file: (see attached file)
Expected result:
No compilation warning.
- - -
Actual result:
Multiple of the following warning:
warning: deprecated conversion from string constant to 'char*' [-Wwrite-strings]
Using the very common fix of prepending (char*) to the string constant is
especially harmful here, because of ZEND_INI_ENTRY3_EX using sizeof() on in: it
returns 4/8 (32/64bits platform).
Using a cast to char[] solves the problem, but the above fix is a very very
common mistake.
If the target fields were const char*, no compilation warning would be rose.
- - -
Solve the problem 17 + 23 = ?: 40 (I even checked the answer using a
calculator)
Submit: Send bug report (I'm going mad, really...)
--
Olivier Favre
Software engineer
Yakaz
www.yakaz.com
--- zend_ini.h 2011-09-30 10:18:59.630952034 +0200
+++ /usr/include/php5/Zend/zend_ini.h 2011-09-29 12:24:39.012882527 +0200
@@ -65,14 +65,14 @@
struct _zend_ini_entry {
int module_number;
int modifiable;
- const char *name;
+ char *name;
uint name_length;
ZEND_INI_MH((*on_modify));
void *mh_arg1;
void *mh_arg2;
void *mh_arg3;
- const char *value;
+ char *value;
uint value_length;
char *orig_value;
--- zend_modules.h 2011-09-30 10:19:54.234949674 +0200
+++ /usr/include/php5/Zend/zend_modules.h 2011-09-12 09:44:01.0 +0200
@@ -98,7 +98,7 @@
unsigned char type;
void *handle;
int module_number;
- const char *build_id;
+ char *build_id;
};
#define MODULE_DEP_REQUIRED 1
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Cast trap with INI entries compiling C++ extension
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 |
Re: [PHP-DEV] Cast trap with INI entries compiling C++ extension
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 Ángel González keis...@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 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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Cast trap with INI entries compiling C++ extension
On 29/09/11 17:42, Olivier Favre wrote: 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. Is it? I never remember which one is a mutable pointer of const chars and which the const pointer of mutable chars so I relied on the description at http://stackoverflow.com/questions/162480/const-in-c#answer-162504 before sending the mail. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Cast trap with INI entries compiling C++ extension
On Thu, 29 Sep 2011 18:13:34 +0100, Ángel González keis...@gmail.com wrote: On 29/09/11 17:42, Olivier Favre wrote: 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. Is it? I never remember which one is a mutable pointer of const chars and which the const pointer of mutable chars so I relied on the description at http://stackoverflow.com/questions/162480/const-in-c#answer-162504 before sending the mail. const char * and char const * are the same (just like const int and int const are the same); what's not the same is char *const. -- Gustavo Lopes -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Cast trap with INI entries compiling C++ extension
On 09/29/2011 07:13 PM, Ángel González wrote: On 29/09/11 17:42, Olivier Favre wrote: 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. Is it? I never remember which one is a mutable pointer of const chars and which the const pointer of mutable chars so I relied on the description at http://stackoverflow.com/questions/162480/const-in-c#answer-162504 before sending the mail. it's easy, whatever const is closer to is immutable const char * is a pointer to a const char, because the const is closer to the char than to the pointer. char * const is a const pointer to a char, because the const is closer to the pointer than to the char. Anyway, never used char const *, which will conflict with my explanation :) Best, Andrey -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Cast trap with INI entries compiling C++ extension
Gustavo Lopes wrote:
const char * and char const * are the same (just like const int and
int const are the same); what's not the same is char *const.
Andrey Hristov wrote:
it's easy, whatever const is closer to is immutable
const char * is a pointer to a const char, because the const is closer
to the char than to the pointer.
char * const is a const pointer to a char, because the const is closer
to the pointer than to the char.
Anyway, never used char const *, which will conflict with my
explanation :)
Best,
Andrey
I stand corrected, char const * is the same as const char* [1], so any
of them would
work for the ini struct.
The C++ FAQ Lite recommends reading it right to left [2], but then the
leftmost const
case is left undefined by the mnemonic, so it isn't valid for all cases,
either.
I'm also attaching a small program showing the different ways of
accessing each of those
and the expected warnings so that your favorite compiler can yell at you :)
[1] http://www.parashift.com/c++-faq/const-correctness.html#faq-18.9
[2] http://www.parashift.com/c++-faq/const-correctness.html#faq-18.5
int main() {
const char* char_const1 = ABC;
char_const1[0] = 'B'; /* Error */
char_const1 = DEF; /* Ok */
char const* char_const2 = ABC;
char_const2[0] = 'B'; /* Error */
char_const2 = DEF; /* Ok */
char * const ptr_const = ABC;
ptr_const[0] = 'B'; /* Ok */
ptr_const = DEF; /* Error */
char const * const const_const = ABC;
const_const[0] = 'B'; /* Error */
const_const = DEF; /* Error */
return 0;
}
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
