Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:27169
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain googlemail.com designates 66.249.82.239 as permitted sender)
DomainKey-Status: good
DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws;
        s=beta; d=googlemail.com;
        h=received:message-id:date:from:user-agent:mime-version:to:subject:references:in-reply-to:content-type;
        b=miZDey20Cwq2MJbyx9fC8CSDGEiq71MigpPKmOLwrX3mtYRzx3AMGJH3umlz0vUnjLJxN720VVR0a0h9MgyXY2KnQ0SR209mowJiMuS0RGMSibgqKw+8t2vpuA1EEdYliPqjYtvLF8U7MOGGrfrKsjdJl1WYaVSHGjQjsW3TKqI=
Message-ID: <458AA06F.7080105@googlemail.com>
Date: Thu, 21 Dec 2006 14:55:43 +0000
User-Agent: Thunderbird 1.5.0.9 (Windows/20061207)
MIME-Version: 1.0
To: PHP internals list <internals@lists.php.net>
References: <45897A41.5060804@googlemail.com>
In-Reply-To: <45897A41.5060804@googlemail.com>
Content-Type: multipart/mixed;
 boundary="------------040407090707060708000505"
Subject: Re: Query on assert_options() api
From: andy.wharmby@googlemail.com (Andy Wharmby)

--------------040407090707060708000505
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

Andy Wharmby wrote:
> Hi Internals
>      A colleague of mine is currently working on creating new test 
> cases to improve the coverage of the PHP test cases and whilst 
> attempting to
> write new tests for the assert extension hit on a problem when 
> attempting to query the current setting of  ASSERT_CALLBACK. using the
> assert_options() api.
>
> With the following simple testcase :
>
> <?php
>
>  function andy()
>  {
>     echo "andy called\n";
>  }
>
>
>  $o = assert_options(ASSERT_CALLBACK, "andy");  /* SET */
>  $n = assert_options(ASSERT_CALLBACK);          /* INQ */
>  echo "old setting is $o\n";
>  echo "new setting is $n\n";  ?>
>
> The expected result was  that the old (if any)  and new setting of the 
> callback function name would be displayed  but the actual results was
>
>        old setting is 1
>        new setting is 1
>
> Looking at the code this is no surprise as the code in assert.c which 
> processes these requests unconditionally returns with RETURN_TRUE.
> For all other assert options other than assert_options() function 
> returns the old value of the specified option as documented in the PHP 
> manual.
>
> Further investigation uncovered that the code actually did behave in 
> the way we expected until it was changed to accept  the array(&$obj, 
> "methodname")
> syntax back in July 2001 (revision 1.32 of assert.c). At this time the 
> return was changed to be unconditionally TRUE.
>
> Unfortunately this makes writing a test case to query the current 
> setting of the ASSERT_CALLBACK option impossible as assert_options() 
> no longer  returns
> any useable information for this option,  i.e to code a testcase as 
> above that sets a value and then checks its set as expected.
>
> If the code is modified as in the attached patch to instead return the 
> ZVAL for the ASSERT_CALLBACK option then we are able create the new 
> testcase to
> verify the  assert_options() api. However, I am concerned there is a 
> good reason this was not done when the code was changed back in 
> 2001.Can anyone think
> of a good reason why returning the zval on this api is not a good idea ?
>
> Any comments on my proposed fix appreciated.
>
> Regards
>    Andy
>
> Andy Wharmby
> IBM United Kingdom Limited
>
Hi All,
      I have now completed further testing on the patch I posted 
yesterday and have uncovered a further defect that will need addressing 
if the
earlier fix is accepted.

With the code modified such that assert_options(ASSERT_CALLBACK) returns 
the current setting of the option it uncovers a problem with
 a recent change to assert.c to fix defect 39718 ( 
http://bugs.php.net/bug.php?id=39718).

With the following simple script

?php

function andy()
{
    echo "andy called";
}

function default_callback()
{
    echo "default_calback called";         
}

   /* Check php.ini setting set */   
  $o = assert_options(ASSERT_CALLBACK);
  echo "Initial setting is \"$o\"   ...it should be default_callback!!!\n";
 
  assert(0);
 
  /* modify callback */
  $o= assert_options(ASSERT_CALLBACK, "andy");
 
  assert(0);
 
?>

and the following php.ini setting:

    assert.callback="default_callback"

then the expected result is:

Initial setting is "default_callback"   ...it should be default_callback!!!
default_calback called

Warning: assert(): Assertion failed in 
C:\Eclipse-PHP\workspace\Testcases\assertbug.php on line 16
andy called

Warning: assert(): Assertion failed in 
C:\Eclipse-PHP\workspace\Testcases\assertbug.php on line 20

the actual result though is

Initial setting is ""   ...it should be default_callback!!!
default_calback called

Warning: assert(): Assertion failed in 
C:\Eclipse-PHP\workspace\Testcases\assert
bug.php on line 16
andy called

Warning: assert(): Assertion failed in 
C:\Eclipse-PHP\workspace\Testcases\assert
bug.php on line 20

Rather than getting the php.ini setting of "default_callback" returned 
NULL is returned instead. Issuing assert() calls the expected callback 
OK though.

The problem is that the value of the global ASSERTG(cb)  is not copied 
to ASSERTG(callback) until the first call to assert() by a request
so if the script includes a query on the setting BEFORE the first call 
to assert() then the value returned is the default INI setting, i.e. 
NULL, rather
than any value specified in php.ini. The issues is easily addressed by 
moving the code that populates ASSERTG(callback) to a new RINIT function
in assert.c.

The modified patches for all the necessary changes to assert.c and 
basic_functions.c are attached. If the fix is accepted I will get my 
colleague to drop the
updated PHPT tests into CVS.

If a defect needs to be opened for this issue just let me know and I 
will do so.

Regards
    Andy

Andy Wharmby
IBM United Kingdom Limited





--------------040407090707060708000505
Content-Type: text/plain;
 name="assert.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="assert.txt"

--- assert.c.old	2006-12-03 17:30:54.000000000 +0000
+++ assert.c	2006-12-21 13:26:35.718750000 +0000
@@ -114,6 +114,17 @@
 	return SUCCESS;
 }
 
+PHP_RINIT_FUNCTION(assert)
+{
+   if (ASSERTG(cb)) {
+		MAKE_STD_ZVAL(ASSERTG(callback));
+		ZVAL_STRING(ASSERTG(callback), ASSERTG(cb), 1);
+	}
+
+   return SUCCESS;
+}
+
+
 PHP_RSHUTDOWN_FUNCTION(assert)
 {
 	if (ASSERTG(callback)) { 
@@ -187,11 +198,6 @@
 		RETURN_TRUE;
 	}
 
-	if (!ASSERTG(callback) && ASSERTG(cb)) {
-		MAKE_STD_ZVAL(ASSERTG(callback));
-		ZVAL_STRING(ASSERTG(callback), ASSERTG(cb), 1);
-	}
-
 	if (ASSERTG(callback)) {
 		zval *args[3];
 		zval *retval;
@@ -286,6 +292,11 @@
 		break;
 
 	case ASSERT_CALLBACK:
+		if (ASSERTG(callback) != NULL) {
+			RETVAL_ZVAL(ASSERTG(callback), 1, 0);
+		} else {
+			RETVAL_NULL();
+		}
 		if (ac == 2) {
 			if (ASSERTG(callback)) {
 				zval_ptr_dtor(&ASSERTG(callback));
@@ -293,7 +304,7 @@
 			ASSERTG(callback) = *value;
 			zval_add_ref(value);
 		}
-		RETURN_TRUE;
+		return;
 		break;
 
 	default:

--------------040407090707060708000505
Content-Type: text/plain;
 name="basic_functions.txt"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="basic_functions.txt"

--- basic_functions.c.old	2006-12-20 11:30:58.000000000 +0000
+++ basic_functions.c	2006-12-21 10:52:33.750000000 +0000
@@ -4132,6 +4132,7 @@
 #ifdef HAVE_SYSLOG_H
 	PHP_RINIT(syslog)(INIT_FUNC_ARGS_PASSTHRU);
 #endif
+	PHP_RINIT(assert) (INIT_FUNC_ARGS_PASSTHRU);
 	PHP_RINIT(dir)(INIT_FUNC_ARGS_PASSTHRU);
 	PHP_RINIT(url_scanner_ex)(INIT_FUNC_ARGS_PASSTHRU);
 

--------------040407090707060708000505--