Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-02-03 Thread Andi Gutmans
Fixed in CVS.
zend_bailout() should work fine and shouldn't leak file descriptors because 
the opened file is in the CG(open_files) list.

Andi

At 01:28 PM 2/1/2004 -0500, Ilia Alshanetsky wrote:
On February 01, 2004 01:08 pm, you wrote:
 Oh OK. As I mentioned in a previous email I'm not sure this is the best way
 of doing the patch. It should be possible to bailout when the parse error
 is detected.
That was my initial concern as well, however the problem is that any earlier
attempts to bail out on the parse error result in leaked file handle. For
example, commenting out E_PARSE error handling in PHP's error handling allows
the behaviour to be addressed without even touch ZE code. But, the
consequence is that the opened filehandle is leaked.
The only other solutions (least code change) is to apply the following patch
(reverting the original). It seems to handle the problem adequatly.
Index: zend_language_scanner.l
===
RCS file: /repository/Zend/Attic/zend_language_scanner.l,v
retrieving revision 1.54.2.25
diff -u -3 -p -r1.54.2.25 zend_language_scanner.l
--- zend_language_scanner.l 29 Nov 2003 19:05:59 -  1.54.2.25
+++ zend_language_scanner.l 1 Feb 2004 18:27:39 -
@@ -383,8 +383,7 @@ ZEND_API zend_op_array *compile_file(zen
zend_do_return(retval_znode, 0 TSRMLS_CC);
CG(in_compilation) = original_in_compilation;
if (compiler_result==1) { /* parser error */
-   CG(unclean_shutdown) = 1;
-   retval = NULL;
+   zend_bailout();
}
compilation_successful=1;
 #ifdef ZEND_MULTIBYTE
Ilia
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php


Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-02-01 Thread Ilia Alshanetsky
The basic problem is as follows. If you have a parse error inside an included 
or required file, the execution stops just for that file and continues for 
the main script. The result is that normally a fatal (parse) error becomes a 
warning. Consequently, it may result in undefined behavior since whatever 
code was inside the included file was not entirely parsed and executed.

Ilia

On January 31, 2004 04:17 pm, you wrote:
 I think I missed out on the original problem. What does the fix, fix? Isn't
 a parse error automatically a fatal error?
 If you revert your patch what won't work?
 Andi

 At 09:59 AM 1/30/2004 -0500, Ilia Alshanetsky wrote:
 It seems like the only way to distinguish between a parse error and a
 non-existant file for regular include() is by doing a zend_stream_open()
  upon failure to determine if the file is avaliable. If it is, then we
  return a parse error and if it does not we continue execution. This does
  add a small overhead for failed includes, but IMHO if a non-existant
  files are being included performance is not a big consideration.
 
 That said, if there is much opposition to the approach I would be happy to
 revert the code to the previous state.
 
 Ilia
 
 P.S. Suggested 'fix' is attached.
 
 On January 30, 2004 05:29 am, Rasmus Lerdorf wrote:
   Ilia, I think there is a problem with your latest fixes on the 4_3
   branch. Stuff that used to work is now broken.  Stuff like this:
  
   main.php:
  
  include 'inc1.inc';
  
   inc1.inc:
  
  @include 'inc2.inc';
  
   If inc2.inc does not exist we now get an error similar to:
  
   Warning: main(./lang/serendipity_lang_.inc.php): failed to open stream:
   No such file or directory in /var/www/s9y/serendipity_lang.inc.php on
   line 5
  
   Warning: main(): Failed opening './inc2.inc' for
   inclusion (include_path='.:/usr/local/lib/php') on line {the include
   line #}
  
   Fatal error: Parse error inside included file. in
   /var/www/htdocs/inc1.inc on line {the include line #}
  
   Remember that it is ok for an include to not find the file.  We issue a
   warning and move on.  It should in no way be treated as a fatal error.
  
   -Rasmus
 
 --
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-02-01 Thread Ilia Alshanetsky
On February 01, 2004 12:30 pm, Andi Gutmans wrote:
 I understand from the comments here that your patch wasn't very successful.

Original patch was incomplete, therefor I made 2 seperate patches (1 for php5 
 1 for php4) that augment the behaviour and fix the problem originally 
reported by Rasmus.

 Any chance you can:
 a) Revert the patch.
 b) Send me a short reproducing example of the combination which doesn't
 currently work? I tried to include a file which has a parse error and it
 did stop execution right away (which it should). I guess I didn't quite
 understand which combination doesn't work.

main.php
?php
include(a.php);
echo Hello World\n;
?

a.php
?php
$a = '
?

The old behaviour would print the parse error message, BUT also the Hello 
World string indicating the execution of the main script was not stopped. My 
argument, which others seem to share is that it should've stopped. If you 
disagree with this, then I'll revert the patch, otherwise the additions (see 
attached) should be applied, which would properly handle file not found 
situation of include().


Ilia


 Thanks,

 Andi

 At 02:04 AM 2/1/2004 -0500, Ilia Alshanetsky wrote:
 The basic problem is as follows. If you have a parse error inside an
  included or required file, the execution stops just for that file and
  continues for the main script. The result is that normally a fatal
  (parse) error becomes a warning. Consequently, it may result in undefined
  behavior since whatever code was inside the included file was not
  entirely parsed and executed.
 
 Ilia
 
 On January 31, 2004 04:17 pm, you wrote:
   I think I missed out on the original problem. What does the fix, fix?
   Isn't a parse error automatically a fatal error?
   If you revert your patch what won't work?
   Andi
  
   At 09:59 AM 1/30/2004 -0500, Ilia Alshanetsky wrote:
   It seems like the only way to distinguish between a parse error and a
   non-existant file for regular include() is by doing a
zend_stream_open() upon failure to determine if the file is
avaliable. If it is, then we return a parse error and if it does not
we continue execution. This does add a small overhead for failed
includes, but IMHO if a non-existant files are being included
performance is not a big consideration.
   
   That said, if there is much opposition to the approach I would be
happy to revert the code to the previous state.
   
   Ilia
   
   P.S. Suggested 'fix' is attached.
   
   On January 30, 2004 05:29 am, Rasmus Lerdorf wrote:
 Ilia, I think there is a problem with your latest fixes on the 4_3
 branch. Stuff that used to work is now broken.  Stuff like this:

 main.php:

include 'inc1.inc';

 inc1.inc:

@include 'inc2.inc';

 If inc2.inc does not exist we now get an error similar to:

 Warning: main(./lang/serendipity_lang_.inc.php): failed to open
 stream: No such file or directory in
 /var/www/s9y/serendipity_lang.inc.php on line 5

 Warning: main(): Failed opening './inc2.inc' for
 inclusion (include_path='.:/usr/local/lib/php') on line {the
 include line #}

 Fatal error: Parse error inside included file. in
 /var/www/htdocs/inc1.inc on line {the include line #}

 Remember that it is ok for an include to not find the file.  We
 issue a warning and move on.  It should in no way be treated as a
 fatal error.

 -Rasmus
   
   --
   PHP Internals - PHP Runtime Development Mailing List
   To unsubscribe, visit: http://www.php.net/unsub.php
Index: zend_execute.c
===
RCS file: /repository/Zend/Attic/zend_execute.c,v
retrieving revision 1.316.2.28
diff -u -3 -p -r1.316.2.28 zend_execute.c
--- zend_execute.c  30 Jan 2004 02:22:31 -  1.316.2.28
+++ zend_execute.c  30 Jan 2004 18:37:21 -
@@ -2152,7 +2152,24 @@ send_by_ref:
case ZEND_REQUIRE:
new_op_array = 
compile_filename(EX(opline)-op2.u.constant.value.lval, inc_filename TSRMLS_CC);
if (!new_op_array) {
-   zend_error(E_ERROR, 
Parse error inside included file.);
+   /* small optimization 
for require() */
+   if (EX(opline)-opcode 
== ZEND_REQUIRE) {
+   goto 
fatal_error;
+   } else {
+   /* This check 
is needed to ensure that included file has a parse error 
+* and we are 
no dealing with a non-existant 

Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-02-01 Thread Andi Gutmans
At 12:42 PM 2/1/2004 -0500, Ilia Alshanetsky wrote:
On February 01, 2004 12:30 pm, Andi Gutmans wrote:
 I understand from the comments here that your patch wasn't very successful.
Original patch was incomplete, therefor I made 2 seperate patches (1 for php5
 1 for php4) that augment the behaviour and fix the problem originally
reported by Rasmus.
Oh OK. As I mentioned in a previous email I'm not sure this is the best way 
of doing the patch. It should be possible to bailout when the parse error 
is detected.

 Any chance you can:
 a) Revert the patch.
 b) Send me a short reproducing example of the combination which doesn't
 currently work? I tried to include a file which has a parse error and it
 did stop execution right away (which it should). I guess I didn't quite
 understand which combination doesn't work.
main.php
?php
include(a.php);
echo Hello World\n;
?
a.php
?php
$a = '
?
The old behaviour would print the parse error message, BUT also the Hello
World string indicating the execution of the main script was not stopped. My
argument, which others seem to share is that it should've stopped. If you
disagree with this, then I'll revert the patch, otherwise the additions (see
attached) should be applied, which would properly handle file not found
situation of include().
I agree completely that this is a bug. When I tried to reproduce I printed 
1 and didn't see it, but I re-ran my test and looked a bit closer.

Andi

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php


Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-02-01 Thread Ilia Alshanetsky
On February 01, 2004 01:08 pm, you wrote:
 Oh OK. As I mentioned in a previous email I'm not sure this is the best way
 of doing the patch. It should be possible to bailout when the parse error
 is detected.

That was my initial concern as well, however the problem is that any earlier 
attempts to bail out on the parse error result in leaked file handle. For 
example, commenting out E_PARSE error handling in PHP's error handling allows 
the behaviour to be addressed without even touch ZE code. But, the 
consequence is that the opened filehandle is leaked. 

The only other solutions (least code change) is to apply the following patch 
(reverting the original). It seems to handle the problem adequatly.

Index: zend_language_scanner.l
===
RCS file: /repository/Zend/Attic/zend_language_scanner.l,v
retrieving revision 1.54.2.25
diff -u -3 -p -r1.54.2.25 zend_language_scanner.l
--- zend_language_scanner.l 29 Nov 2003 19:05:59 -  1.54.2.25
+++ zend_language_scanner.l 1 Feb 2004 18:27:39 -
@@ -383,8 +383,7 @@ ZEND_API zend_op_array *compile_file(zen
zend_do_return(retval_znode, 0 TSRMLS_CC);
CG(in_compilation) = original_in_compilation;
if (compiler_result==1) { /* parser error */
-   CG(unclean_shutdown) = 1;
-   retval = NULL;
+   zend_bailout();
}
compilation_successful=1;
 #ifdef ZEND_MULTIBYTE

Ilia

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-31 Thread Andi Gutmans
BTW take a look at zenderror(). This function is called when there's a 
parse error.
It leads to an E_PARSE error and setting EG(exit_status) to 255.
If there is a problem, and I haven't quite understood what the problem is 
today, that might be the place to fix it in.

Andi

At 11:17 PM 1/31/2004 +0200, Andi Gutmans wrote:
I think I missed out on the original problem. What does the fix, fix? 
Isn't a parse error automatically a fatal error?
If you revert your patch what won't work?
Andi

At 09:59 AM 1/30/2004 -0500, Ilia Alshanetsky wrote:
It seems like the only way to distinguish between a parse error and a
non-existant file for regular include() is by doing a zend_stream_open() upon
failure to determine if the file is avaliable. If it is, then we return a
parse error and if it does not we continue execution. This does add a small
overhead for failed includes, but IMHO if a non-existant files are being
included performance is not a big consideration.
That said, if there is much opposition to the approach I would be happy to
revert the code to the previous state.
Ilia

P.S. Suggested 'fix' is attached.

On January 30, 2004 05:29 am, Rasmus Lerdorf wrote:
 Ilia, I think there is a problem with your latest fixes on the 4_3 branch.
 Stuff that used to work is now broken.  Stuff like this:

 main.php:

include 'inc1.inc';

 inc1.inc:

@include 'inc2.inc';

 If inc2.inc does not exist we now get an error similar to:

 Warning: main(./lang/serendipity_lang_.inc.php): failed to open stream: No
 such file or directory in /var/www/s9y/serendipity_lang.inc.php on line 5

 Warning: main(): Failed opening './inc2.inc' for
 inclusion (include_path='.:/usr/local/lib/php') on line {the include line
 #}

 Fatal error: Parse error inside included file. in
 /var/www/htdocs/inc1.inc on line {the include line #}

 Remember that it is ok for an include to not find the file.  We issue a
 warning and move on.  It should in no way be treated as a fatal error.

 -Rasmus
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php


[PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Rasmus Lerdorf
Ilia, I think there is a problem with your latest fixes on the 4_3 branch.
Stuff that used to work is now broken.  Stuff like this:

main.php:

   include 'inc1.inc';

inc1.inc:

   @include 'inc2.inc';

If inc2.inc does not exist we now get an error similar to:

Warning: main(./lang/serendipity_lang_.inc.php): failed to open stream: No
such file or directory in /var/www/s9y/serendipity_lang.inc.php on line 5

Warning: main(): Failed opening './inc2.inc' for
inclusion (include_path='.:/usr/local/lib/php') on line {the include line #}

Fatal error: Parse error inside included file. in
/var/www/htdocs/inc1.inc on line {the include line #}

Remember that it is ok for an include to not find the file.  We issue a
warning and move on.  It should in no way be treated as a fatal error.

-Rasmus

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Derick Rethans
On Fri, 30 Jan 2004, Rasmus Lerdorf wrote:

 Remember that it is ok for an include to not find the file.  We issue a
 warning and move on.  It should in no way be treated as a fatal error.

But it should on a parse error in an include file, not?

Derick

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Rasmus Lerdorf
On Fri, 30 Jan 2004, Derick Rethans wrote:

 On Fri, 30 Jan 2004, Rasmus Lerdorf wrote:

  Remember that it is ok for an include to not find the file.  We issue a
  warning and move on.  It should in no way be treated as a fatal error.

 But it should on a parse error in an include file, not?

Yes, but a failed include is not a parse error.

-Rasmus

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Derick Rethans
On Fri, 30 Jan 2004, Rasmus Lerdorf wrote:

 On Fri, 30 Jan 2004, Derick Rethans wrote:

  On Fri, 30 Jan 2004, Rasmus Lerdorf wrote:
 
   Remember that it is ok for an include to not find the file.  We issue a
   warning and move on.  It should in no way be treated as a fatal error.
 
  But it should on a parse error in an include file, not?

 Yes, but a failed include is not a parse error.

I know, I was just making sure while fixing it :)

Derick

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Ilia Alshanetsky
It seems like the only way to distinguish between a parse error and a 
non-existant file for regular include() is by doing a zend_stream_open() upon 
failure to determine if the file is avaliable. If it is, then we return a 
parse error and if it does not we continue execution. This does add a small 
overhead for failed includes, but IMHO if a non-existant files are being 
included performance is not a big consideration.

That said, if there is much opposition to the approach I would be happy to 
revert the code to the previous state.

Ilia

P.S. Suggested 'fix' is attached.

On January 30, 2004 05:29 am, Rasmus Lerdorf wrote:
 Ilia, I think there is a problem with your latest fixes on the 4_3 branch.
 Stuff that used to work is now broken.  Stuff like this:

 main.php:

include 'inc1.inc';

 inc1.inc:

@include 'inc2.inc';

 If inc2.inc does not exist we now get an error similar to:

 Warning: main(./lang/serendipity_lang_.inc.php): failed to open stream: No
 such file or directory in /var/www/s9y/serendipity_lang.inc.php on line 5

 Warning: main(): Failed opening './inc2.inc' for
 inclusion (include_path='.:/usr/local/lib/php') on line {the include line
 #}

 Fatal error: Parse error inside included file. in
 /var/www/htdocs/inc1.inc on line {the include line #}

 Remember that it is ok for an include to not find the file.  We issue a
 warning and move on.  It should in no way be treated as a fatal error.

 -Rasmus
Index: zend_execute.c
===
RCS file: /repository/ZendEngine2/zend_execute.c,v
retrieving revision 1.592
diff -u -3 -p -r1.592 zend_execute.c
--- zend_execute.c  30 Jan 2004 02:22:17 -  1.592
+++ zend_execute.c  30 Jan 2004 14:56:23 -
@@ -3390,7 +3390,13 @@ int zend_include_or_eval_handler(ZEND_OP
case ZEND_REQUIRE:
new_op_array = 
compile_filename(EX(opline)-op2.u.constant.value.lval, inc_filename TSRMLS_CC);
if (!new_op_array) {
-   zend_error(E_ERROR, Parse error inside included 
file.);
+   zend_file_handle file_handle;
+   zend_bool can_open = 
zend_stream_open(inc_filename-value.str.val, file_handle TSRMLS_CC);
+   zend_file_handle_dtor(file_handle);
+
+   if (can_open != SUCCESS) {
+   zend_error(E_ERROR, Parse error inside 
included file.);
+   }
}
break;
case ZEND_EVAL: {

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Cesare D'Amico
Alle 15:59, venerdì 30 gennaio 2004, Ilia Alshanetsky ha scritto:
 +   if (can_open != SUCCESS) {

I didn't try to apply/compile the patch, I just read it, so perhaps I'm 
wrong, but... shouldn't that != be a == ?

-- 
Cesare D'Amico - boss (@t) cesaredamico (dot) com
http://www.cesaredamico.com~   http://www.phpday.it

The best way to accelerate a Windows machine is at 9.81 m/s^2

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Rasmus Lerdorf
On Fri, 30 Jan 2004, Ilia Alshanetsky wrote:
 It seems like the only way to distinguish between a parse error and a 
 non-existant file for regular include() is by doing a zend_stream_open() upon 
 failure to determine if the file is avaliable. If it is, then we return a 
 parse error and if it does not we continue execution. This does add a small 
 overhead for failed includes, but IMHO if a non-existant files are being 
 included performance is not a big consideration.

Granted, I haven't looked closely at the code, but PHP knows that failure 
to open an include file is an E_WARNING not an E_ERROR.  An E_WARNING 
should never cause script execution to terminate.  If we already know it 
is non-fatal, why is the extra check needed?

-Rasmus

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Ilia Alshanetsky
On January 30, 2004 12:03 pm, Rasmus Lerdorf wrote:
 Granted, I haven't looked closely at the code, but PHP knows that failure
 to open an include file is an E_WARNING not an E_ERROR.  An E_WARNING
 should never cause script execution to terminate.  If we already know it
 is non-fatal, why is the extra check needed?

A parse error is an interesting beast because unlike other fatal errors it is 
not handled by PHP's error handler inside main.c. It is not done there since 
that would leak the stdio stream opened in ZE. Inside the regular 
require/include we have very little data to see what happened, we only know 
what op_array is NULL. Whether this happend due to a parse error or a not 
found file is unknown. So there are 3 possibilities @ this point:
- included file was not found
- included file was found, but has a parse error
- required file was found, but has a parse error
(if requires was was not found, ZE would've already bailed out).

Since we do not know which of the 3 occured, although I suppose the require 
situation can be optimized to: 
if (!new_op_array  ... == ZEND_REQUIRE)

For include situation we need to see if the error is due to the file not being 
there, in which case we can go forward with execution, or if the problem was 
due to a parse error, in which case we need to stop. The easiest way I saw of 
doing it (without modifying too much existing code) was to perform a 
zend_stream_open(), which would check if the file is avaliable.

Ilia

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Rasmus Lerdorf
On Fri, 30 Jan 2004, Ilia Alshanetsky wrote:
 For include situation we need to see if the error is due to the file not being 
 there, in which case we can go forward with execution, or if the problem was 
 due to a parse error, in which case we need to stop. The easiest way I saw of 
 doing it (without modifying too much existing code) was to perform a 
 zend_stream_open(), which would check if the file is avaliable.

Since this only happens on an error condition I guess it is ok.  Just 
seems like we should be able to set some kind of hint in Zend to pass 
this information back up.

-Rasmus

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Ilia Alshanetsky
Here is the final revision of the patch for the situation, if there are no 
last minute objections, it'll go in the CVS and we'll proceed with RC2.

Ilia

On January 30, 2004 12:37 pm, Rasmus Lerdorf wrote:
 On Fri, 30 Jan 2004, Ilia Alshanetsky wrote:
  For include situation we need to see if the error is due to the file not
  being there, in which case we can go forward with execution, or if the
  problem was due to a parse error, in which case we need to stop. The
  easiest way I saw of doing it (without modifying too much existing code)
  was to perform a zend_stream_open(), which would check if the file is
  avaliable.

 Since this only happens on an error condition I guess it is ok.  Just
 seems like we should be able to set some kind of hint in Zend to pass
 this information back up.

 -Rasmus
Index: zend_execute.c
===
RCS file: /repository/ZendEngine2/zend_execute.c,v
retrieving revision 1.592
diff -u -3 -p -r1.592 zend_execute.c
--- zend_execute.c  30 Jan 2004 02:22:17 -  1.592
+++ zend_execute.c  30 Jan 2004 18:16:32 -
@@ -3390,7 +3390,24 @@ int zend_include_or_eval_handler(ZEND_OP
case ZEND_REQUIRE:
new_op_array = 
compile_filename(EX(opline)-op2.u.constant.value.lval, inc_filename TSRMLS_CC);
if (!new_op_array) {
-   zend_error(E_ERROR, Parse error inside included 
file.);
+   /* small optimization for require() */
+   if (EX(opline)-op2.u.constant.value.lval == 
ZEND_REQUIRE) {
+   goto fatal_error;
+   } else {
+   /* This check is needed to ensure that 
included file has a parse error 
+* and we are no dealing with a non-existant 
include, which is not considered
+* to be a fatal error.
+*/
+   zend_file_handle file_handle;
+   zend_bool can_open = 
zend_stream_open(inc_filename-value.str.val, file_handle TSRMLS_CC);
+   zend_file_handle_dtor(file_handle);
+
+   /* file open succeeded but still no op-array, 
likely parse error */
+   if (can_open == SUCCESS) {
+fatal_error:
+   zend_error(E_ERROR, Parse error 
inside included file.);
+   }
+   }
}
break;
case ZEND_EVAL: {

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Rasmus Lerdorf
On Fri, 30 Jan 2004, Ilia Alshanetsky wrote:
 Here is the final revision of the patch for the situation, if there are no 
 last minute objections, it'll go in the CVS and we'll proceed with RC2.

Uh, that is a ZE2 patch.  We are talking about PHP 4.3.5 RC2 here.

-Rasmus

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Ilia Alshanetsky
Here is the ZE1 revision.

Ilia

On January 30, 2004 01:21 pm, you wrote:
 On Fri, 30 Jan 2004, Ilia Alshanetsky wrote:
  Here is the final revision of the patch for the situation, if there are
  no last minute objections, it'll go in the CVS and we'll proceed with
  RC2.

 Uh, that is a ZE2 patch.  We are talking about PHP 4.3.5 RC2 here.

 -Rasmus
Index: zend_execute.c
===
RCS file: /repository/Zend/Attic/zend_execute.c,v
retrieving revision 1.316.2.28
diff -u -3 -p -r1.316.2.28 zend_execute.c
--- zend_execute.c  30 Jan 2004 02:22:31 -  1.316.2.28
+++ zend_execute.c  30 Jan 2004 18:37:21 -
@@ -2152,7 +2152,24 @@ send_by_ref:
case ZEND_REQUIRE:
new_op_array = 
compile_filename(EX(opline)-op2.u.constant.value.lval, inc_filename TSRMLS_CC);
if (!new_op_array) {
-   zend_error(E_ERROR, 
Parse error inside included file.);
+   /* small optimization 
for require() */
+   if (EX(opline)-opcode 
== ZEND_REQUIRE) {
+   goto 
fatal_error;
+   } else {
+   /* This check 
is needed to ensure that included file has a parse error 
+* and we are 
no dealing with a non-existant include, which is not considered
+* to be a 
fatal error.
+*/
+   
zend_file_handle file_handle = {0};
+   int can_open = 
(zend_open(inc_filename-value.str.val, file_handle) == SUCCESS  
ZEND_IS_VALID_FILE_HANDLE(file_handle));
+   
zend_file_handle_dtor(file_handle);
+
+   /* file open 
succeeded but still no op-array, likely parse error */
+   if (can_open) {
+fatal_error:
+   
zend_error(E_ERROR, Parse error inside included file.);
+   }
+   }
}
break;
case ZEND_EVAL: {

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Sterling Hughes
 + zend_file_handle file_handle;
 + zend_bool can_open = 
 zend_stream_open(inc_filename-value.str.val, file_handle TSRMLS_CC);
 + zend_file_handle_dtor(file_handle);
 +

wouldn't a stat be better suited here?

-sterling

 + /* file open succeeded but still no op-array, 
 likely parse error */
 + if (can_open == SUCCESS) {
 +fatal_error:
 + zend_error(E_ERROR, Parse error 
 inside included file.);
 + }
 + }
   }
   break;
   case ZEND_EVAL: {
 

 -- 
 PHP Internals - PHP Runtime Development Mailing List
 To unsubscribe, visit: http://www.php.net/unsub.php

-- 
Reductionists like to take things apart.  The rest of us are
 just trying to get it together.
- Larry Wall, Programming Perl, 3rd Edition

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Wez Furlong
stat wouldn't work for URL includes.

  + zend_file_handle file_handle;
  + zend_bool can_open = zend_stream_open(inc_filename-value.str.val,
file_handle TSRMLS_CC);
  + zend_file_handle_dtor(file_handle);
  +

 wouldn't a stat be better suited here?

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Martin Jansen
On Fri Jan 30, 2004 at 01:3752PM -0500, Ilia Alshanetsky wrote:
 + 
 zend_error(E_ERROR, Parse error inside included file.);

Is there any chance to include the name of the included file here?

-- 
- Martin   Martin Jansen
http://martinjansen.com/

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-DEV] Warnings in include files suddenly treated as fatal

2004-01-30 Thread Ilia Alshanetsky
The oirignal parse error messages not only tells you the filename but also the 
line where the parse error occurs. The 2nd error (the one you referring to) 
will tell you on which line the 'broken' file is being included.

Ex:

Parse error: parse error in /home/rei/PHP_CVS/php4/BUILD/inc2.inc on line 3

Fatal error: Parse error inside included file. 
in /home/rei/PHP_CVS/php4/BUILD/inc1.php on line 2

Ilia

On January 30, 2004 02:14 pm, Martin Jansen wrote:
 On Fri Jan 30, 2004 at 01:3752PM -0500, Ilia Alshanetsky wrote:
  +   
  zend_error(E_ERROR, Parse error inside included file.);

 Is there any chance to include the name of the included file here?

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php