Skip to content
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

Jump to bottom

fix Imagick::count() with PHP 5.6.0RC2 #35

Merged
merged 3 commits into from
Jul 23, 2014
Merged

fix Imagick::count() with PHP 5.6.0RC2 #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ca019ad
fix Imagick::count() with PHP 5.6.0RC2
remicollet Jul 23, 2014
1330250
add the optional mode (not used)
remicollet Jul 23, 2014
dedcf84
no need for conditional, count_elements exists (at least) since 5.0.0
remicollet Jul 23, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions imagick.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1340,6 +1340,12 @@ PHP_IMAGICK_API zend_class_entry *php_imagickpixel_get_class_entry()
ZEND_END_ARG_INFO()
#endif

#if PHP_VERSION_ID >= 50600
ZEND_BEGIN_ARG_INFO_EX(imagick_count_args, 0, 0, 0)
ZEND_ARG_INFO(0, mode)
ZEND_END_ARG_INFO()
#endif

/* ImagickDraw */
#if MagickLibVersion > 0x649
ZEND_BEGIN_ARG_INFO_EX(imagickdraw_settextkerning_args, 0, 0, 1)
Expand Down Expand Up @@ -2161,7 +2167,11 @@ static zend_function_entry php_imagick_class_methods[] =
#endif
PHP_ME(imagick, __construct, imagick_construct_args, ZEND_ACC_PUBLIC|ZEND_ACC_CTOR)
PHP_ME(imagick, __tostring, NULL, ZEND_ACC_PUBLIC)
#if PHP_VERSION_ID >= 50600
PHP_ME(imagick, count, imagick_count_args, ZEND_ACC_PUBLIC)
#else
PHP_ME(imagick, count, imagick_zero_args, ZEND_ACC_PUBLIC)
#endif
PHP_ME(imagick, getpixeliterator, imagick_zero_args, ZEND_ACC_PUBLIC)
PHP_ME(imagick, getpixelregioniterator, imagick_getpixelregioniterator_args, ZEND_ACC_PUBLIC)
PHP_ME(imagick, readimage, imagick_readimage_args, ZEND_ACC_PUBLIC)
Expand Down Expand Up @@ -2708,6 +2718,17 @@ static void php_imagick_init_globals(zend_imagick_globals *imagick_globals)
imagick_globals->progress_monitor = 0;
}

static int php_imagick_count_elements(zval *object, long *count TSRMLS_DC) /* {{{ */
{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the PR - but this is the wrong way round isn't it? Looking at the array handling code if HAVE_SPL is defined, the code will go through the existing 'count' function.

It's only if HAVE_SPL is not defined that the php_imagick_count_elements would need to be bound to imagick_object_handlers.count_elements right? Or am I misreading it?

btw the extra patch to revert the BC break to calling count is probably going to be applied, but I will also correct the method in here as it should be following the interface definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, no need for conditional here.
This handler exists at least in 5.0.0

From my understanding, tests

  • count($a) => if the handler is defined, it is used, else the count() method is called (with 1 arg in 5.6, which explain this issue as no arg as expected).
  • $a->count() => the method is called.

So, this means that ca019ad OR 1330250 is enough to fix the issue.

php_imagick_object *intern= (php_imagick_object *)zend_object_store_get_object(object TSRMLS_CC);

if (intern->magick_wand) {
*count = MagickGetNumberImages(intern->magick_wand);
return SUCCESS;
}
return FAILURE;
}

#if PHP_VERSION_ID < 50399
static zval *php_imagick_read_property(zval *object, zval *member, int type TSRMLS_DC)
#else
Expand Down Expand Up @@ -2933,6 +2954,7 @@ PHP_MINIT_FUNCTION(imagick)
ce.create_object = php_imagick_object_new;
imagick_object_handlers.clone_obj = php_imagick_clone_imagick_object;
imagick_object_handlers.read_property = php_imagick_read_property;
imagick_object_handlers.count_elements = php_imagick_count_elements;
php_imagick_sc_entry = zend_register_internal_class(&ce TSRMLS_CC);
#if defined(HAVE_SPL)
zend_class_implements(php_imagick_sc_entry TSRMLS_CC, 2, zend_ce_iterator, spl_ce_Countable);
Expand Down
3 changes: 2 additions & 1 deletion imagick_class.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2988,8 +2988,9 @@ PHP_METHOD(imagick, __tostring)
PHP_METHOD(imagick, count)
{
php_imagick_object *intern;
long mode = 0;

if (zend_parse_parameters_none() == FAILURE) {
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|l", &mode) == FAILURE) {
return;
}

Expand Down