Hi devs,
this night i saw that the BC of strrpos()/strripos()
is broken by a patch commited by pollita 7 months ago :
http://cvs.php.net/diff.php/php-src/ext/standard/string.c?login=2&r1=1.370&r2=1.371&ty=u
The documenation of both function states if the second parameter is an
integer instead of string then its value is used as a ord of the character
to be used during the search. This no more true with HEAD.
A script to reproduce :
<?php
var_dump(strrpos("ABCDEF","DEF"));
var_dump(strrpos("ABCDEF","DAF"));
var_dump(strrpos("ABCDEF",ord("D")));
?>
HEAD :
int(3)
bool(false)
bool(false)
PHP 4.3.5-dev (cli) (built: Dec 2 2003 00:13:37) (DEBUG):
int(3)
int(3)
int(3)
What do you think? Is it ok to break the BC here or not? As you see the
other change introduced with the patch is to use the whole needle string
while searching, not only the first character.
Comments are welcome,
Andrey
this night i saw that the BC of
strrpos()/strripos()
is broken by a patch commited by pollita 7 months ago :
http://cvs.php.net/diff.php/php-src/ext/standard/string.c?login=2&r1=1.370&r2=1.371&ty=u
The documenation of both function states if the second parameter is an
integer instead of string then its value is used as a ord of the character
to be used during the search. This no more true with HEAD.
Good catch. This should not have been broken, I'll fix this.
What do you think? Is it ok to break the BC here or not? As you see the
other change introduced with the patch is to use the whole needle string
while searching, not only the first character.
That break was intentional. Having strrpos() behave differently from
strpos() in regards to needle size was not a good thing.
-Sara
Sara Golemon wrote:
The documenation of both function states if the second parameter is an
integer instead of string then its value is used as a ord of the character
to be used during the search. This no more true with HEAD.Good catch. This should not have been broken, I'll fix this.
okie
What do you think? Is it ok to break the BC here or not? As you see the
other change introduced with the patch is to use the whole needle string
while searching, not only the first character.That break was intentional. Having
strrpos()behave differently from
strpos() in regards to needle size was not a good thing.
In this case it should be stated somewhere maybe in the still imaginery
"the_thin_differences_between_PHP4_and_PHP5.txt"
-Sara
Andrey
What do you think? Is it ok to break the BC here or not? As you see the
other change introduced with the patch is to use the whole needle string
while searching, not only the first character.That break was intentional. Having
strrpos()behave differently from
strpos() in regards to needle size was not a good thing.In this case it should be stated somewhere maybe in the still imaginery
"the_thin_differences_between_PHP4_and_PHP5.txt"
Well, it's in the manual page (down at the bottom). (Note: Ignore the note
about converting integers to strings, I've taken that out in light of
undoing that particular BC breakage). But you're absolutely right, a spot
in a yet-to-be-writen FAQ couldn't hurt.
While I was in that file I fixed an offset reporting issue (offset didn't
exist in 4.3 so it's not something to MFH) and introduced an optimized
version of the string search for single character needles. In strrpos()
it'll only be a minor difference, but strripos() saves a pair of needless
emallocs.
-Sara