When calculating ceil($pager_total_items[$element] / $limit); for a mini pager which is equal to -9223372036854775808 by running $ php -r "echo (int) ceil(PHP_INT_MAX/1);"

we need to either fix the ceil code in

function pager_default_initialize($total, $limit, $element = 0) {
  global $pager_page_array, $pager_total, $pager_total_items, $pager_limits;

  $page = pager_find_page($element);

  // We calculate the total of pages as ceil(items / limit).
  $pager_total_items[$element] = $total;
  $pager_total[$element] = ceil($pager_total_items[$element] / $limit);

or fix the initialization of $total to PHP_MAX_INT / 2 in which seems better.

class Mini
...
  public function postExecute(&$result) {
    ...
    pager_default_initialize(PHP_INT_MAX / 2, $this->get_items_per_page(), $this->options['id']);
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

clemens.tolboom’s picture

Code seems to be added to core through #1901290: Replace the mini pager with a lite pager (which does not run a count query) which is rather new Feb 18, 2013.

jibran’s picture

Issue tags: +VDC

Tagging.

clemens.tolboom’s picture

I've rewritten the mini page test to accommodate this failure.

clemens.tolboom’s picture

Issue tags: +VDC

Restoring the VDC tag.

dawehner’s picture

FileSize
2.98 KB

Thanks for fixing the issue. I used your tests to provide a separate test coverage just to ensure that it's working both with 3 and 1 items per page,
because 1 item per page actually feels a bit like an edge case.

clemens.tolboom’s picture

We have to decide what's the best maximum. I did some more testing

$ php -r "echo (int) ceil((PHP_INT_MAX-512)/1);"
9223372036854774784

$ php -r "echo (int) ceil((PHP_INT_MAX-511)/1);"
-9223372036854775808

so we are safe using PHP_INT_MAX / 2 but is that ok?

dawehner’s picture

Your current bit is consistent on all php versions: http://3v4l.org/gE3gE

The main thing I care about at that point: Make the piece of code understandable to we don't run into the same problem again.

damiankloip’s picture

Status: Needs review » Reviewed & tested by the community

I think I agree with keeping it simple, and this method will work just fine too.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 262895e and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.