Skip to content

Support negative string offset #100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

Support negative string offset #100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
052a075
Update the zend_fetch_dimension_address_read to support negative stri…
Easen Jun 4, 2012
f5a0a21
Moved the variable declaration to the top of the block
Easen Jun 11, 2012
2dd2eda
Added the unit test for negative string offset, setting a value
Easen Jun 17, 2012
410f749
Updated zend_assign_to_string_offset to support negative offset
Easen Jun 17, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions Zend/tests/str_offset_001.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ function foo($x) {
}

$str = "abc";
var_dump($str[-4]);
var_dump($str[-3]);
var_dump($str[-2]);
var_dump($str[-1]);
var_dump($str[0]);
var_dump($str[1]);
Expand All @@ -15,6 +18,9 @@ var_dump($str[3]);
var_dump($str[1][0]);
var_dump($str[2][1]);

foo($str[-4]);
foo($str[-3]);
foo($str[-2]);
foo($str[-1]);
foo($str[0]);
foo($str[1]);
Expand All @@ -24,11 +30,14 @@ foo($str[1][0]);
foo($str[2][1]);
?>
--EXPECTF--
Notice: Uninitialized string offset: -1 in %sstr_offset_001.php on line %d
Notice: Uninitialized string offset: -4 in %sstr_offset_001.php on line %d
string(0) ""
string(1) "a"
string(1) "b"
string(1) "c"
string(1) "a"
string(1) "b"
string(1) "c"

Notice: Uninitialized string offset: 3 in %sstr_offset_001.php on line %d
string(0) ""
Expand All @@ -37,11 +46,14 @@ string(1) "b"
Notice: Uninitialized string offset: 1 in %sstr_offset_001.php on line %d
string(0) ""

Notice: Uninitialized string offset: -1 in %sstr_offset_001.php on line %d
Notice: Uninitialized string offset: -4 in %sstr_offset_001.php on line %d
string(0) ""
string(1) "a"
string(1) "b"
string(1) "c"
string(1) "a"
string(1) "b"
string(1) "c"

Notice: Uninitialized string offset: 3 in %sstr_offset_001.php on line %d
string(0) ""
Expand Down
18 changes: 18 additions & 0 deletions Zend/tests/str_offset_003.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
--TEST--
string offset 003
--FILE--
<?php
$a = "aaa";
var_dump($a);
$a[-1] = "b";
var_dump($a);
$a[-2] = "b";
var_dump($a);
$a[-3] = "b";
var_dump($a);
?>
--EXPECTF--
string(3) "aaa"
string(3) "aab"
string(3) "abb"
string(3) "bbb"
15 changes: 10 additions & 5 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,13 +751,12 @@ static inline void zend_assign_to_object(zval **retval, zval **object_ptr, zval
FREE_OP_IF_VAR(free_value);
}

static inline int zend_assign_to_string_offset(const temp_variable *T, const zval *value, int value_type TSRMLS_DC)
static inline int zend_assign_to_string_offset(temp_variable *T, const zval *value, int value_type TSRMLS_DC)
{
if (Z_TYPE_P(T->str_offset.str) == IS_STRING) {

if (((int)T->str_offset.offset < 0)) {
zend_error(E_WARNING, "Illegal string offset: %d", T->str_offset.offset);
return 0;
T->str_offset.offset += Z_STRLEN_P(T->str_offset.str);
}

if (T->str_offset.offset >= Z_STRLEN_P(T->str_offset.str)) {
Expand Down Expand Up @@ -1268,6 +1267,7 @@ static void zend_fetch_dimension_address_read(temp_variable *result, zval **cont
case IS_STRING: {
zval tmp;
zval *ptr;
long ldim;

if (Z_TYPE_P(dim) != IS_LONG) {
switch(Z_TYPE_P(dim)) {
Expand Down Expand Up @@ -1302,15 +1302,20 @@ static void zend_fetch_dimension_address_read(temp_variable *result, zval **cont
INIT_PZVAL(ptr);
Z_TYPE_P(ptr) = IS_STRING;

if (Z_LVAL_P(dim) < 0 || Z_STRLEN_P(container) <= Z_LVAL_P(dim)) {
ldim = Z_LVAL_P(dim);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't you make it cache the strlen result so it does not have to compute the string length twice? I don't know how strlen is computed, but I just figured it would be good practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Z_STRLEN_P is not a function but a macro to the string length value stored within the variable structure, so they's no unnecessary computation.

zend_operators.h:

#define Z_STRLEN(zval) (zval).value.str.len
...
#define Z_STRLEN_P(zval_p) Z_STRLEN(*zval_p)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have taken a greater look at the PHP source before asking that, sorry.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hey

if (ldim < 0) {
ldim += Z_STRLEN_P(container);
}

if (ldim < 0 || Z_STRLEN_P(container) <= ldim) {
if (type != BP_VAR_IS) {
zend_error(E_NOTICE, "Uninitialized string offset: %ld", Z_LVAL_P(dim));
}
Z_STRVAL_P(ptr) = STR_EMPTY_ALLOC();
Z_STRLEN_P(ptr) = 0;
} else {
Z_STRVAL_P(ptr) = (char*)emalloc(2);
Z_STRVAL_P(ptr)[0] = Z_STRVAL_P(container)[Z_LVAL_P(dim)];
Z_STRVAL_P(ptr)[0] = Z_STRVAL_P(container)[ldim];
Z_STRVAL_P(ptr)[1] = 0;
Z_STRLEN_P(ptr) = 1;
}
Expand Down
2 changes: 0 additions & 2 deletions tests/lang/bug22592.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ var_dump($result);
$e = $result[1] = $result[6];
var_dump($result);
var_dump($a, $b, $c, $d, $e);
$result[-1] = 'a';
?>
--EXPECT--
string(5) "* *-*"
Expand All @@ -50,4 +49,3 @@ string(1) "s"
string(1) "4"
string(1) "5"
string(1) "5"
[Illegal string offset: -1]