Skip to content

Commit c757c61

Browse files
GirgiasGirgias
committed
Remove unnecessary error handler replacement in SPL
Document why it is needed in the remaining cases Drive-by refactoring Closes GH-6955
1 parent 9eb295f commit c757c61

File tree

2 files changed

+29
-46
lines changed

2 files changed

+29
-46
lines changed

‎ext/spl/spl_directory.c

Lines changed: 28 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@ static inline int spl_filesystem_is_dot(const char * d_name) /* {{{ */
267267
/* }}} */
268268

269269
/* {{{ spl_filesystem_dir_open */
270-
/* open a directory resource */
270+
/* open a directory resource
271+
* Can emit an E_WARNING as it reports errors from php_stream_opendir() */
271272
static void spl_filesystem_dir_open(spl_filesystem_object* intern, zend_string *path)
272273
{
273274
int skip_dots = SPL_HAS_FLAG(intern->flags, SPL_FILE_DIR_SKIPDOTS);
@@ -297,7 +298,9 @@ static void spl_filesystem_dir_open(spl_filesystem_object* intern, zend_string *
297298
}
298299
/* }}} */
299300

300-
static int spl_filesystem_file_open(spl_filesystem_object *intern, int use_include_path, int silent) /* {{{ */
301+
/* Can generate E_WARNINGS as we report errors from stream initialized via
302+
* php_stream_open_wrapper_ex() */
303+
static zend_result spl_filesystem_file_open(spl_filesystem_object *intern, bool use_include_path) /* {{{ */
301304
{
302305
zval tmp;
303306

@@ -441,7 +444,6 @@ static spl_filesystem_object *spl_filesystem_object_create_info(spl_filesystem_o
441444
{
442445
spl_filesystem_object *intern;
443446
zval arg1;
444-
zend_error_handling error_handling;
445447

446448
if (!file_path || !ZSTR_LEN(file_path)) {
447449
#ifdef PHP_WIN32
@@ -450,8 +452,6 @@ static spl_filesystem_object *spl_filesystem_object_create_info(spl_filesystem_o
450452
return NULL;
451453
}
452454

453-
zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling);
454-
455455
ce = ce ? ce : source->info_class;
456456

457457
intern = spl_filesystem_from_obj(spl_filesystem_object_new_ex(ce));
@@ -465,7 +465,6 @@ static spl_filesystem_object *spl_filesystem_object_create_info(spl_filesystem_o
465465
spl_filesystem_info_set_filename(intern, file_path);
466466
}
467467

468-
zend_restore_error_handling(&error_handling);
469468
return intern;
470469
} /* }}} */
471470

@@ -556,8 +555,9 @@ static spl_filesystem_object *spl_filesystem_object_create_type(int num_args, sp
556555
intern->u.file.open_mode = zend_string_copy(open_mode);
557556
intern->u.file.zcontext = resource;
558557

558+
/* spl_filesystem_file_open() can generate E_WARNINGs which we want to promote to exceptions */
559559
zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling);
560-
if (spl_filesystem_file_open(intern, use_include_path, 0) == FAILURE) {
560+
if (spl_filesystem_file_open(intern, use_include_path) == FAILURE) {
561561
zend_restore_error_handling(&error_handling);
562562
zval_ptr_dtor(return_value);
563563
ZVAL_NULL(return_value);
@@ -733,6 +733,7 @@ void spl_filesystem_object_construct(INTERNAL_FUNCTION_PARAMETERS, zend_long cto
733733
}
734734
intern->flags = flags;
735735

736+
/* spl_filesystem_dir_open() may emit an E_WARNING */
736737
zend_replace_error_handling(EH_THROW, spl_ce_UnexpectedValueException, &error_handling);
737738
#ifdef HAVE_GLOB
738739
if (SPL_HAS_FLAG(ctor_flags, DIT_CTOR_GLOB) && memcmp(ZSTR_VAL(path), "glob://", sizeof("glob://")-1) != 0) {
@@ -745,10 +746,9 @@ void spl_filesystem_object_construct(INTERNAL_FUNCTION_PARAMETERS, zend_long cto
745746
spl_filesystem_dir_open(intern, path);
746747

747748
}
749+
zend_restore_error_handling(&error_handling);
748750

749751
intern->u.dir.is_recursive = instanceof_function(intern->std.ce, spl_ce_RecursiveDirectoryIterator) ? 1 : 0;
750-
751-
zend_restore_error_handling(&error_handling);
752752
}
753753
/* }}} */
754754

@@ -1225,17 +1225,13 @@ PHP_METHOD(SplFileInfo, getLinkTarget)
12251225
spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS);
12261226
ssize_t ret;
12271227
char buff[MAXPATHLEN];
1228-
zend_error_handling error_handling;
12291228

12301229
if (zend_parse_parameters_none() == FAILURE) {
12311230
RETURN_THROWS();
12321231
}
12331232

1234-
zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling);
1235-
12361233
if (intern->file_name == NULL) {
12371234
if (spl_filesystem_object_get_file_name(intern) != SUCCESS) {
1238-
zend_restore_error_handling(&error_handling);
12391235
RETURN_THROWS();
12401236
}
12411237
}
@@ -1247,7 +1243,6 @@ PHP_METHOD(SplFileInfo, getLinkTarget)
12471243
if (!IS_ABSOLUTE_PATH(ZSTR_VAL(intern->file_name), ZSTR_LEN(intern->file_name))) {
12481244
char expanded_path[MAXPATHLEN];
12491245
if (!expand_filepath_with_mode(ZSTR_VAL(intern->file_name), expanded_path, NULL, 0, CWD_EXPAND )) {
1250-
zend_restore_error_handling(&error_handling);
12511246
php_error_docref(NULL, E_WARNING, "No such file or directory");
12521247
RETURN_FALSE;
12531248
}
@@ -1268,8 +1263,6 @@ PHP_METHOD(SplFileInfo, getLinkTarget)
12681263

12691264
RETVAL_STRINGL(buff, ret);
12701265
}
1271-
1272-
zend_restore_error_handling(&error_handling);
12731266
}
12741267
/* }}} */
12751268

@@ -1279,17 +1272,13 @@ PHP_METHOD(SplFileInfo, getRealPath)
12791272
spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS);
12801273
char buff[MAXPATHLEN];
12811274
char *filename;
1282-
zend_error_handling error_handling;
12831275

12841276
if (zend_parse_parameters_none() == FAILURE) {
12851277
RETURN_THROWS();
12861278
}
12871279

1288-
zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling);
1289-
12901280
if (intern->type == SPL_FS_DIR && !intern->file_name && intern->u.dir.entry.d_name[0]) {
12911281
if (spl_filesystem_object_get_file_name(intern) != SUCCESS) {
1292-
zend_restore_error_handling(&error_handling);
12931282
RETURN_THROWS();
12941283
}
12951284
}
@@ -1311,8 +1300,6 @@ PHP_METHOD(SplFileInfo, getRealPath)
13111300
} else {
13121301
RETVAL_FALSE;
13131302
}
1314-
1315-
zend_restore_error_handling(&error_handling);
13161303
}
13171304
/* }}} */
13181305

@@ -2061,29 +2048,31 @@ PHP_METHOD(SplFileObject, __construct)
20612048
RETURN_THROWS();
20622049
}
20632050

2064-
zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling);
2065-
20662051
intern->u.file.open_mode = zend_string_copy(open_mode);
2067-
if (spl_filesystem_file_open(intern, use_include_path, 0) == SUCCESS) {
2068-
path_len = strlen(intern->u.file.stream->orig_path);
20692052

2070-
if (path_len > 1 && IS_SLASH_AT(intern->u.file.stream->orig_path, path_len-1)) {
2071-
path_len--;
2072-
}
2053+
/* spl_filesystem_file_open() can generate E_WARNINGs which we want to promote to exceptions */
2054+
zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling);
2055+
zend_result retval = spl_filesystem_file_open(intern, use_include_path);
2056+
zend_restore_error_handling(&error_handling);
2057+
if (retval == FAILURE) {
2058+
RETURN_THROWS();
2059+
}
20732060

2074-
while (path_len > 1 && !IS_SLASH_AT(intern->u.file.stream->orig_path, path_len-1)) {
2075-
path_len--;
2076-
}
2061+
path_len = strlen(intern->u.file.stream->orig_path);
20772062

2078-
if (path_len) {
2079-
path_len--;
2080-
}
2063+
if (path_len > 1 && IS_SLASH_AT(intern->u.file.stream->orig_path, path_len-1)) {
2064+
path_len--;
2065+
}
20812066

2082-
intern->path = zend_string_init(intern->u.file.stream->orig_path, path_len, 0);
2067+
while (path_len > 1 && !IS_SLASH_AT(intern->u.file.stream->orig_path, path_len-1)) {
2068+
path_len--;
20832069
}
20842070

2085-
zend_restore_error_handling(&error_handling);
2071+
if (path_len) {
2072+
path_len--;
2073+
}
20862074

2075+
intern->path = zend_string_init(intern->u.file.stream->orig_path, path_len, 0);
20872076
} /* }}} */
20882077

20892078
/* {{{ Construct a new temp file object */
@@ -2108,8 +2097,9 @@ PHP_METHOD(SplTempFileObject, __construct)
21082097
intern->file_name = file_name;
21092098
intern->u.file.open_mode = zend_string_init("wb", sizeof("wb")-1, 0);
21102099

2100+
/* spl_filesystem_file_open() can generate E_WARNINGs which we want to promote to exceptions */
21112101
zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling);
2112-
if (spl_filesystem_file_open(intern, 0, 0) == SUCCESS) {
2102+
if (spl_filesystem_file_open(intern, /* use_include_path */ false) == SUCCESS) {
21132103
intern->path = ZSTR_EMPTY_ALLOC();
21142104
}
21152105
zend_string_release(file_name);

‎ext/spl/spl_iterators.c

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,6 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
486486
zval *iterator;
487487
zend_class_entry *ce_iterator;
488488
zend_long mode, flags;
489-
zend_error_handling error_handling;
490489
zval caching_it, aggregate_retval;
491490

492491
switch (rit_type) {
@@ -500,7 +499,6 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
500499
RETURN_THROWS();
501500
}
502501

503-
zend_replace_error_handling(EH_THROW, spl_ce_InvalidArgumentException, &error_handling);
504502
if (instanceof_function(Z_OBJCE_P(iterator), zend_ce_aggregate)) {
505503
zend_function **getiterator_cache = Z_OBJCE_P(iterator)->iterator_funcs_ptr
506504
? &Z_OBJCE_P(iterator)->iterator_funcs_ptr->zf_new_iterator : NULL;
@@ -525,7 +523,6 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
525523
RETURN_THROWS();
526524
}
527525

528-
zend_replace_error_handling(EH_THROW, spl_ce_InvalidArgumentException, &error_handling);
529526
if (instanceof_function(Z_OBJCE_P(iterator), zend_ce_aggregate)) {
530527
zend_function **getiterator_cache = Z_OBJCE_P(iterator)->iterator_funcs_ptr
531528
? &Z_OBJCE_P(iterator)->iterator_funcs_ptr->zf_new_iterator : NULL;
@@ -542,7 +539,6 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
542539
zval_ptr_dtor(iterator);
543540
}
544541
zend_throw_exception(spl_ce_InvalidArgumentException, "An instance of RecursiveIterator or IteratorAggregate creating it is required", 0);
545-
zend_restore_error_handling(&error_handling);
546542
return;
547543
}
548544

@@ -592,8 +588,6 @@ static void spl_recursive_it_it_construct(INTERNAL_FUNCTION_PARAMETERS, zend_cla
592588
intern->iterators[0].haschildren = NULL;
593589
intern->iterators[0].getchildren = NULL;
594590

595-
zend_restore_error_handling(&error_handling);
596-
597591
if (EG(exception)) {
598592
zend_object_iterator *sub_iter;
599593

@@ -1382,11 +1376,9 @@ static spl_dual_it_object* spl_dual_it_construct(INTERNAL_FUNCTION_PARAMETERS, z
13821376
return NULL;
13831377
}
13841378
intern->dit_type = DIT_AppendIterator;
1385-
zend_replace_error_handling(EH_THROW, spl_ce_InvalidArgumentException, &error_handling);
13861379
object_init_ex(&intern->u.append.zarrayit, spl_ce_ArrayIterator);
13871380
zend_call_method_with_0_params(Z_OBJ(intern->u.append.zarrayit), spl_ce_ArrayIterator, &spl_ce_ArrayIterator->constructor, "__construct", NULL);
13881381
intern->u.append.iterator = spl_ce_ArrayIterator->get_iterator(spl_ce_ArrayIterator, &intern->u.append.zarrayit, 0);
1389-
zend_restore_error_handling(&error_handling);
13901382
return intern;
13911383
case DIT_RegexIterator:
13921384
case DIT_RecursiveRegexIterator: {
@@ -1405,6 +1397,7 @@ static spl_dual_it_object* spl_dual_it_construct(INTERNAL_FUNCTION_PARAMETERS, z
14051397
return NULL;
14061398
}
14071399

1400+
/* pcre_get_compiled_regex_cache() might emit E_WARNINGs that we want to promote to exception */
14081401
zend_replace_error_handling(EH_THROW, spl_ce_InvalidArgumentException, &error_handling);
14091402
intern->u.regex.pce = pcre_get_compiled_regex_cache(regex);
14101403
zend_restore_error_handling(&error_handling);

0 commit comments

Comments
 (0)