Re: [PHP-CVS] cvs: php-src(PHP_5_3) /ext/pdo_sqlite sqlite_statement.c
Ilia Alshanetsky ha scritto: > iliaa Tue May 19 19:15:18 2009 UTC > > Modified files: (Branch: PHP_5_3) > /php-src/ext/pdo_sqlite sqlite_statement.c > Log: > The \0 removal is only needed prior to 3.4.0 I hate to do this (and as a php-src newbie I'm a bit afraid to), but once again this is simply wrong. The commit breaks compatibility with sqlite < 3.4.0. You are trying to fix something that has never been broken. The sqlite3.h comment was wrong and the missing parens were leading you to think that the issue existed and was being worked around in PHP. Otherwise PDO_SQLITE would have been returning strings with trailing '\0' since Feb 2005 to June 2007, which is quite unlikely. See: http://cvs.php.net/viewvc.cgi/php-src/ext/pdo_sqlite/sqlite_statement.c?r1=1.12&r2=1.13&view=patch http://www.sqlite.org/changes.html#version_3_4_0 I've verified the compatibility break by compiling PHP 5.3 with a cvs checkout of sqlite 3.3.8 (-D 2007-01-01). Here's the result of one of the many failing tests: TEST 6/14 [ext/pdo_sqlite/tests/bug46139.phpt] DIFF 002+ 'fo' 002- 'foo' 004+ 'fo' 004- 'foo' 006+ 'fo' 006- 'foo' DONE I hope this is enough proof to back up my theory and for you to revert your latest changes. Cheers -- Matteo Beccati OpenX - http://www.openx.org -- PHP CVS Mailing List (http://www.php.net/) To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src(PHP_5_3) /ext/pdo_sqlite sqlite_statement.c
it looks like in recent version of sqlite3 the behavior had changed
some what. I am going to check which version of sqlite3 had followed
the old behavior and put the code into the appropriate define block.
Ilia Alshanetsky
On 19-May-09, at 1:33 PM, Matteo Beccati wrote:
Ilia Alshanetsky ha scritto:
*len is a pointer to an integer, that is being decremented to
reduce the
overall length of the string to "remove" the NUL terminator. If you
remove the operation you'll end up with 2 NULs at the end of the
string.
As I said, I coulndn't find any reference to the fact that "the NUL
terminator is included in the byte count for TEXT values" in
sqlite3.h.
I don't want to argue about this, but here's a small test case that
hopefully proves that the code you added back is just useless, as it
doesn't do what you'd expect by reading the comment.
$ cat len.c; gcc -Wall len.c; ./a.out
#include
#include
void decrease_len(int *len)
{
*len--;
}
int main(int argc, char **argv)
{
int len = 10;
decrease_len(&len);
assert(len == 9);
return 0;
}
len.c: In function ‘decrease_len’:
len.c:6: warning: value computed is not used
a.out: len.c:14: main: Assertion `len == 9' failed.
Aborted
Cheers
--
Matteo Beccati
OpenX - http://www.openx.org
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src(PHP_5_3) /ext/pdo_sqlite sqlite_statement.c
The code should be (*len)--; then the result is as desired, I'll adjust.
Ilia Alshanetsky
On 19-May-09, at 1:33 PM, Matteo Beccati wrote:
Ilia Alshanetsky ha scritto:
*len is a pointer to an integer, that is being decremented to
reduce the
overall length of the string to "remove" the NUL terminator. If you
remove the operation you'll end up with 2 NULs at the end of the
string.
As I said, I coulndn't find any reference to the fact that "the NUL
terminator is included in the byte count for TEXT values" in
sqlite3.h.
I don't want to argue about this, but here's a small test case that
hopefully proves that the code you added back is just useless, as it
doesn't do what you'd expect by reading the comment.
$ cat len.c; gcc -Wall len.c; ./a.out
#include
#include
void decrease_len(int *len)
{
*len--;
}
int main(int argc, char **argv)
{
int len = 10;
decrease_len(&len);
assert(len == 9);
return 0;
}
len.c: In function ‘decrease_len’:
len.c:6: warning: value computed is not used
a.out: len.c:14: main: Assertion `len == 9' failed.
Aborted
Cheers
--
Matteo Beccati
OpenX - http://www.openx.org
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src(PHP_5_3) /ext/pdo_sqlite sqlite_statement.c
Ilia Alshanetsky ha scritto:
> *len is a pointer to an integer, that is being decremented to reduce the
> overall length of the string to "remove" the NUL terminator. If you
> remove the operation you'll end up with 2 NULs at the end of the string.
As I said, I coulndn't find any reference to the fact that "the NUL
terminator is included in the byte count for TEXT values" in sqlite3.h.
I don't want to argue about this, but here's a small test case that
hopefully proves that the code you added back is just useless, as it
doesn't do what you'd expect by reading the comment.
$ cat len.c; gcc -Wall len.c; ./a.out
#include
#include
void decrease_len(int *len)
{
*len--;
}
int main(int argc, char **argv)
{
int len = 10;
decrease_len(&len);
assert(len == 9);
return 0;
}
len.c: In function ‘decrease_len’:
len.c:6: warning: value computed is not used
a.out: len.c:14: main: Assertion `len == 9' failed.
Aborted
Cheers
--
Matteo Beccati
OpenX - http://www.openx.org
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src(PHP_5_3) /ext/pdo_sqlite sqlite_statement.c
*len is a pointer to an integer, that is being decremented to reduce
the overall length of the string to "remove" the NUL terminator. If
you remove the operation you'll end up with 2 NULs at the end of the
string.
Ilia Alshanetsky
On 19-May-09, at 12:58 PM, Matteo Beccati wrote:
Hi Ilia,
+ if (*len) {
+/* sqlite3.h says "the NUL terminator is included in the byte
count for TEXT values" */
+*len--; /* do not remove this, even though it generates a
warning */
+ }
Would you mind explaining me why the fix was faulty?
Cheers
--
Matteo Beccati
OpenX - http://www.openx.org
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-CVS] cvs: php-src(PHP_5_3) /ext/pdo_sqlite sqlite_statement.c
Hi Ilia,
> + if (*len) {
> + /* sqlite3.h says "the NUL terminator is
> included in the byte count for TEXT values" */
> + *len--; /* do not remove this, even though it
> generates a warning */
> + }
Would you mind explaining me why the fix was faulty?
Cheers
--
Matteo Beccati
OpenX - http://www.openx.org
--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
