Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:67652
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain brianfrance.com from 208.83.223.140 cause and error)
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Message-ID: <2C9EF127-64C7-4BC6-85ED-1B9A6C6A0470@brianfrance.com>
Date: Fri, 7 Jun 2013 23:33:43 -0400
To: PHP Internals <internals@lists.php.net>
Mime-Version: 1.0 (Mac OS X Mail 6.5 \(1508\))
Subject: Major Compatible Change or Simple Bug fix?
From: brian@brianfrance.com ("Brian J. France")

Was the change that went into 5.3.24 and 5.3.14 a major compatible =
change that should be backed out and put into in 5.5/5.6 or a simple bug =
fix that is ok in a patch release?

Example code:

----  User Space -----

<?php

X_SET_ERROR('NONE');

try=20
{
  $param =3D 1;
  X_FUNC($parm1);
}
catch(Exception $e)=20
{

}

$error =3D X_GET_ERROR();
?>

----  Extension Code -----

<snip>

ZEND_BEGIN_ARG_INFO_EX(X_FUNC_ARGS, 0, 0, 1)
  ZEND_ARG_INFO(0, string)
ZEND_END_ARG_INFO()

<snip>

  PHP_FE(X_FUNC, X_FUNC_ARGS)

<snip>

static char error[100];

PHP_FUNCTION(X_FUNC)
{
  char *value;
  int len;

  if (zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), =
"s",&value, &len) =3D=3D FAILURE) {
    strcpy(error, "X_FUNC: bad params");
    RETURN_FALSE;
  }
}

-----------------------

Assume X_SET_ERROR and X_GET_ERROR are simple set and get of the error =
string.

-----------------------

Running the user space code with the extension installed:

5.3.23 and 5.3.13:  $error =3D=3D 'X_FUNC: bad params'

5.3.24+ and 5.3.14+:  $error =3D=3D 'NONE'


5.3.23 and 5.3.13 the engine would check the params to the function, =
setup an exception, call X_FUNC (and strcpy is called), then detect the =
exception and throw it.

A patch that was added to 5.3.24 and 5.3.14 makes it check the params to =
the function, setup an exception and throw it.  The X_FUNC is never =
called (and strcpy never happens).

Main bug for the patch:

Bug #63914 zend_do_fcall_common_helper_SPEC does not handle exceptions =
properly
  https://bugs.php.net/bug.php?id=3D63914

5.3.24 commit:
  =
http://git.php.net/?p=3Dphp-src.git;a=3Dcommitdiff;h=3Dc4686b4de93a89e8265=
331b0d4a6a7954ccbae95

5.4.14 commit (diff 1 on this)
  =
http://git.php.net/?p=3Dphp-src.git;a=3Dcommitdiff;h=3De9f996c00d5cb6448da=
c1457dd9dae320c16904f


This patch also added a core dump to 5.3.24, see the last comment of =
this bug I opened:

https://bugs.php.net/bug.php?id=3D64966

I added a patch to the bug to fix the core dump (could somebody review =
to make sure it make sense, I don't know the code very well).  With the =
patch in the bug it fixes the core dump, 5.3 and 5.4 act the same.

The question is, should the functionally change be in a patch release as =
it is a minor bug fix or is this a major compatible change that should =
backed out and put into a version like 5.5/5.6?

Thoughts?

Brian