Skip to content

Refactor SplFileObject methods #6014

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 18 commits into from
Closed

Refactor SplFileObject methods #6014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
d5019e8
Drop useless arg2
Girgias Aug 18, 2020
18b2bb7
Declare methods normally instead of using a wrapper macro
Girgias Aug 18, 2020
f2ddf83
Inline FileFunctionCall macro
Girgias Aug 18, 2020
2a7007a
Better fstat
Girgias Aug 18, 2020
00d8899
Better SPL fscanf
Girgias Aug 18, 2020
33a938c
Add todo notes to file.c flock
Girgias Aug 18, 2020
ee0faa0
Better SplObjectFile::flock()
Girgias Aug 18, 2020
e4f7baf
Add standard to dependencies
Girgias Aug 18, 2020
4e90dd5
Explicitly define stupid flock constants
Girgias Aug 18, 2020
218658b
Remove now unused spl_filesystem_file_call()
Girgias Aug 18, 2020
d254071
Use macro to check if object is initialized
Girgias Aug 18, 2020
6004d88
Drop useless includes
Girgias Aug 18, 2020
3977f25
Add test case for SplFileObject::fstat() when fstat() is disabled
Girgias Aug 18, 2020
8a8d8b2
Fix flock_compat layer
Girgias Aug 18, 2020
a3a305a
Extract fstat into a seperate function
Girgias Aug 20, 2020
201e683
Fix config.m4 script
Girgias Aug 24, 2020
1c57b65
Pass spl_filesystem_object pointer to CHECK_SPL_FILE_OBJECT_IS_INITIA…
Girgias Aug 24, 2020
076414b
Drop comments for flock()
Girgias Aug 24, 2020
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
1 change: 1 addition & 0 deletions ext/spl/config.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
PHP_NEW_EXTENSION(spl, php_spl.c spl_functions.c spl_engine.c spl_iterators.c spl_array.c spl_directory.c spl_exceptions.c spl_observer.c spl_dllist.c spl_heap.c spl_fixedarray.c, no,, -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1)
PHP_INSTALL_HEADERS([ext/spl], [php_spl.h spl_array.h spl_directory.h spl_engine.h spl_exceptions.h spl_functions.h spl_iterators.h spl_observer.h spl_dllist.h spl_heap.h spl_fixedarray.h])
PHP_ADD_EXTENSION_DEP(spl, pcre, true)
PHP_ADD_EXTENSION_DEP(spl, standard, true)
225 changes: 89 additions & 136 deletions ext/spl/spl_directory.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@

#include "php.h"
#include "php_ini.h"
#include "ext/standard/info.h"
#include "ext/standard/file.h"
#include "ext/standard/php_filestat.h"
#include "ext/standard/flock_compat.h"
#include "ext/standard/scanf.h"
#include "ext/standard/php_string.h"
#include "zend_compile.h"
#include "zend_exceptions.h"
#include "zend_interfaces.h"

Expand All @@ -35,12 +36,6 @@
#include "spl_directory_arginfo.h"
#include "spl_exceptions.h"

#include "php.h"
#include "fopen_wrappers.h"

#include "ext/standard/basic_functions.h"
#include "ext/standard/php_filestat.h"

#define SPL_HAS_FLAG(flags, test_flag) ((flags & test_flag) ? 1 : 0)

/* declare the class handlers */
Expand All @@ -57,6 +52,13 @@ PHPAPI zend_class_entry *spl_ce_GlobIterator;
PHPAPI zend_class_entry *spl_ce_SplFileObject;
PHPAPI zend_class_entry *spl_ce_SplTempFileObject;

// TODO Use standard Error
#define CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(spl_filesystem_object_pointer) \
if (!(spl_filesystem_object_pointer)->u.file.stream) { \
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized"); \
RETURN_THROWS(); \
}

static void spl_filesystem_file_free_line(spl_filesystem_object *intern) /* {{{ */
{
if (intern->u.file.current_line) {
Expand Down Expand Up @@ -1901,64 +1903,6 @@ static int spl_filesystem_file_read(spl_filesystem_object *intern, int silent) /
return SUCCESS;
} /* }}} */

static int spl_filesystem_file_call(spl_filesystem_object *intern, zend_function *func_ptr, int pass_num_args, zval *return_value, zval *arg2) /* {{{ */
{
zend_fcall_info fci;
zend_fcall_info_cache fcic;
zval *zresource_ptr = &intern->u.file.zresource, *params;
int result;
int num_args = pass_num_args + (arg2 ? 2 : 1);

if (Z_ISUNDEF_P(zresource_ptr)) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
return FAILURE;
}

params = (zval*)safe_emalloc(num_args, sizeof(zval), 0);
params[0] = *zresource_ptr;

if (arg2) {
params[1] = *arg2;
}

if (zend_get_parameters_array_ex(pass_num_args, params + (arg2 ? 2 : 1)) != SUCCESS) {
efree(params);
WRONG_PARAM_COUNT_WITH_RETVAL(FAILURE);
}

fci.size = sizeof(fci);
fci.object = NULL;
fci.retval = return_value;
fci.param_count = num_args;
fci.params = params;
fci.named_params = NULL;
ZVAL_STR(&fci.function_name, func_ptr->common.function_name);

fcic.function_handler = func_ptr;
fcic.called_scope = NULL;
fcic.object = NULL;

result = zend_call_function(&fci, &fcic);

if (result == FAILURE || Z_ISUNDEF_P(return_value)) {
RETVAL_FALSE;
}

efree(params);
return result;
} /* }}} */

#define FileFunctionCall(func_name, pass_num_args, arg2) /* {{{ */ \
{ \
zend_function *func_ptr; \
func_ptr = (zend_function *)zend_hash_str_find_ptr(EG(function_table), #func_name, sizeof(#func_name) - 1); \
if (func_ptr == NULL) { \
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Internal error, function %s() not found. Please report", #func_name); \
return; \
} \
spl_filesystem_file_call(intern, func_ptr, pass_num_args, return_value, arg2); \
} /* }}} */

static int spl_filesystem_file_read_csv(spl_filesystem_object *intern, char delimiter, char enclosure, int escape, zval *return_value) /* {{{ */
{
int ret = SUCCESS;
Expand Down Expand Up @@ -2075,7 +2019,7 @@ static int spl_filesystem_file_read_line(zval * this_ptr, spl_filesystem_object

static void spl_filesystem_file_rewind(zval * this_ptr, spl_filesystem_object *intern) /* {{{ */
{
if(!intern->u.file.stream) {
if (!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
return;
}
Expand Down Expand Up @@ -2203,10 +2147,7 @@ PHP_METHOD(SplFileObject, eof)
RETURN_THROWS();
}

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

RETURN_BOOL(php_stream_eof(intern->u.file.stream));
} /* }}} */
Expand Down Expand Up @@ -2239,10 +2180,7 @@ PHP_METHOD(SplFileObject, fgets)
RETURN_THROWS();
}

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

if (spl_filesystem_file_read(intern, 0) == FAILURE) {
RETURN_FALSE;
Expand All @@ -2259,10 +2197,7 @@ PHP_METHOD(SplFileObject, current)
RETURN_THROWS();
}

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

if (!intern->u.file.current_line && Z_ISUNDEF(intern->u.file.current_zval)) {
spl_filesystem_file_read_line(ZEND_THIS, intern, 1);
Expand Down Expand Up @@ -2382,15 +2317,6 @@ PHP_METHOD(SplFileObject, getChildren)
/* return NULL */
} /* }}} */

/* {{{ FileFunction */
#define FileFunction(func_name) \
PHP_METHOD(SplFileObject, func_name) \
{ \
spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS); \
FileFunctionCall(func_name, ZEND_NUM_ARGS(), NULL); \
}
/* }}} */

/* {{{ Return current line as csv */
PHP_METHOD(SplFileObject, fgetcsv)
{
Expand All @@ -2402,10 +2328,7 @@ PHP_METHOD(SplFileObject, fgetcsv)

if (zend_parse_parameters(ZEND_NUM_ARGS(), "|sss", &delim, &d_len, &enclo, &e_len, &esc, &esc_len) == SUCCESS) {

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

switch(ZEND_NUM_ARGS())
{
Expand Down Expand Up @@ -2570,19 +2493,51 @@ PHP_METHOD(SplFileObject, getCsvControl)
}
/* }}} */

/* {{{ Portable file locking */
FileFunction(flock)
static int flock_values[] = { LOCK_SH, LOCK_EX, LOCK_UN };

/* {{{ Portable file locking, copy pasted from ext/standard/file.c flock() function.
* This is done to prevent this to fail if flock is disabled via disable_functions */
Copy link
Member

Choose a reason for hiding this comment

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

Is that a bug or a feature? If someone disabled flock(), I imagine they also wouldn't want SplFileInfo::flock() to work, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good question, but most SPL methods are not disabled when the corresponding functional version is disabled. That's why I did this, moreover this was considered as an internal "bug" so if we want to revert this I think any function which is disabled should then disable it's SPL counterpart IMHO

Copy link
Member

Choose a reason for hiding this comment

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

Either way, copy and paste doesn't look like a good idea to me.

Copy link
Member

Choose a reason for hiding this comment

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

That's a good question, but most SPL methods are not disabled when the corresponding functional version is disabled.

Good point. If it's the same with other functions, then I'm fine with it.

PHP_METHOD(SplFileObject, flock)
{
spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS);
zval *wouldblock = NULL;
int act;
zend_long operation = 0;

if (zend_parse_parameters(ZEND_NUM_ARGS(), "l|z", &operation, &wouldblock) == FAILURE) {
RETURN_THROWS();
}

CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

act = operation & PHP_LOCK_UN;
if (act < 1 || act > 3) {
zend_argument_value_error(1, "must be either LOCK_SH, LOCK_EX, or LOCK_UN");
RETURN_THROWS();
}

if (wouldblock) {
ZEND_TRY_ASSIGN_REF_LONG(wouldblock, 0);
}

/* flock_values contains all possible actions if (operation & PHP_LOCK_NB) we won't block on the lock */
act = flock_values[act - 1] | (operation & PHP_LOCK_NB ? LOCK_NB : 0);
if (php_stream_lock(intern->u.file.stream, act)) {
if (operation && errno == EWOULDBLOCK && wouldblock) {
ZEND_TRY_ASSIGN_REF_LONG(wouldblock, 1);
}
RETURN_FALSE;
}
RETURN_TRUE;
}
/* }}} */

/* {{{ Flush the file */
PHP_METHOD(SplFileObject, fflush)
{
spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS);

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

RETURN_BOOL(!php_stream_flush(intern->u.file.stream));
} /* }}} */
Expand All @@ -2593,10 +2548,7 @@ PHP_METHOD(SplFileObject, ftell)
spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS);
zend_long ret;

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

ret = php_stream_tell(intern->u.file.stream);

Expand All @@ -2617,10 +2569,7 @@ PHP_METHOD(SplFileObject, fseek)
RETURN_THROWS();
}

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

spl_filesystem_file_free_line(intern);
RETURN_LONG(php_stream_seek(intern->u.file.stream, pos, (int)whence));
Expand All @@ -2633,10 +2582,7 @@ PHP_METHOD(SplFileObject, fgetc)
char buf[2];
int result;

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

spl_filesystem_file_free_line(intern);

Expand All @@ -2660,28 +2606,35 @@ PHP_METHOD(SplFileObject, fpassthru)
{
spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS);

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

RETURN_LONG(php_stream_passthru(intern->u.file.stream));
} /* }}} */

/* {{{ Implements a mostly ANSI compatible fscanf() */
PHP_METHOD(SplFileObject, fscanf)
{
int result, num_varargs = 0;
zend_string *format_str;
zval *varargs= NULL;
spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS);

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
if (zend_parse_parameters(ZEND_NUM_ARGS(), "S*", &format_str, &varargs, &num_varargs) == FAILURE) {
RETURN_THROWS();
}

spl_filesystem_file_free_line(intern);
intern->u.file.current_line_num++;
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

FileFunctionCall(fscanf, ZEND_NUM_ARGS(), NULL);
/* Get next line */
if (spl_filesystem_file_read(intern, 0) == FAILURE) {
RETURN_THROWS();
}

result = php_sscanf_internal(intern->u.file.current_line, ZSTR_VAL(format_str), num_varargs, varargs, 0, return_value);

if (SCAN_ERROR_WRONG_PARAM_COUNT == result) {
WRONG_PARAM_COUNT;
}
}
/* }}} */

Expand All @@ -2698,10 +2651,7 @@ PHP_METHOD(SplFileObject, fwrite)
RETURN_THROWS();
}

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

if (ZEND_NUM_ARGS() > 1) {
if (length >= 0) {
Expand Down Expand Up @@ -2732,10 +2682,7 @@ PHP_METHOD(SplFileObject, fread)
RETURN_THROWS();
}

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

if (length <= 0) {
php_error_docref(NULL, E_WARNING, "Length parameter must be greater than 0");
Expand All @@ -2750,7 +2697,18 @@ PHP_METHOD(SplFileObject, fread)
}

/* {{{ Stat() on a filehandle */
FileFunction(fstat)
PHP_METHOD(SplFileObject, fstat)
{
spl_filesystem_object *intern = Z_SPLFILESYSTEM_P(ZEND_THIS);

if (zend_parse_parameters_none() == FAILURE) {
RETURN_THROWS();
}

CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

php_fstat(intern->u.file.stream, return_value);
}
/* }}} */

/* {{{ Truncate file to 'size' length */
Expand All @@ -2763,10 +2721,7 @@ PHP_METHOD(SplFileObject, ftruncate)
RETURN_THROWS();
}

if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}
CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

if (!php_stream_truncate_supported(intern->u.file.stream)) {
zend_throw_exception_ex(spl_ce_LogicException, 0, "Can't truncate file %s", intern->file_name);
Expand All @@ -2785,10 +2740,8 @@ PHP_METHOD(SplFileObject, seek)
if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &line_pos) == FAILURE) {
RETURN_THROWS();
}
if(!intern->u.file.stream) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0, "Object not initialized");
RETURN_THROWS();
}

CHECK_SPL_FILE_OBJECT_IS_INITIALIZED(intern);

if (line_pos < 0) {
zend_throw_exception_ex(spl_ce_LogicException, 0, "Can't seek file %s to negative line " ZEND_LONG_FMT, intern->file_name, line_pos);
Expand Down
Loading