array_shift() expects parameter 1 to be array, null given in /var/www/likwid/test/www/sites/all/modules/contrib/feeds/includes/FeedsBatch.inc on line 110.

PHP 5.3 is quite intolerant with non initialized variables, and such things, but it forces developers to use some good practices.

This is how I would fix it (file FeedsBatch.inc, line 104):

  /**
   * @return
   *   Next available item or NULL if there is none. Every returned item is
   *   removed from the internal array.
   */
  public function shiftItem() {
    if (! empty($this->items)) {
      return array_shift($this->items);
    }
    return NULL;
  }
CommentFileSizeAuthor
#4 feeds_array_shift_701432.patch1.3 KBWill White
#3 feeds_array_shift_701432.patch837 bytesWill White

Comments

alex_b’s picture

alex_b commented
Title: array_shift() expects parameter 1 to be array, null given in (...) PHP 5.3 » PHP 5.3: array_shift() expects parameter 1 to be array, null given in (...)
Version: 6.x-1.0-alpha10 » 6.x-1.x-dev
Priority: Normal » Critical

Thanks for reporting, needs to be fixed in head. Critical for next release.

Will White’s picture

Will White commented
Status: Active » Needs review

Here's a patch for another solution that sets the items property to array() if NULL is passed to the setter. Not sure which approach is better.

Will White’s picture

Will White commented
StatusFileSize
new feeds_array_shift_701432.patch837 bytes

Forgot the patch!

Will White’s picture

Will White commented
StatusFileSize
new feeds_array_shift_701432.patch1.3 KB

Updated patch that ensures the parser does not set the items property to NULL. Added documentation to setItems() method about how $items must be an array.

pounard’s picture

pounard
Primary language French
commented

Keep NULL, this is better, because the PHPdoc say so. If people started to write code based on this implementation, keep it. Changing the NULL return to an empty array is changing the API signature, you should avoid.

EDIT: Except if you consider you are in early development stage, and accept to break the API for course, this is your code.

alex_b’s picture

alex_b commented

#5:

Keep NULL, this is better, because the PHPdoc say so.

Will did #5 actually according to my advice, I prefer making the API tighter at this point rather than feeling bound to the current doc. That's why we're in alpha mode: the rough API is laid out and stable, small changes to API are OK. Refactor over legacy creep.

In this particular case I'd say we should be clear in what's expected from parsers and keep sanitation to an absolute minimum, it's just going to be code that we need to maintain.

pounard’s picture

pounard
Primary language French
commented

@alex_b No problems, this was a suggestion, agree with you then. Hope it will be stable one day:)

alex_b’s picture

alex_b commented
Status: Needs review » Reviewed & tested by the community

#7: I owe everybody a road map (and I also owe you a response to functioning tests, still on my backlog :) - in the meantime I've started to tag beta critical issues as "beta blocker" http://drupal.org/project/issues/search/feeds?text=&assigned=&submitted=...

This is RTBC.

alex_b’s picture

alex_b commented
Status: Reviewed & tested by the community » Fixed

Committed, thank you for detailed report and repeated patch rolling.

Status: Fixed » Closed (fixed)

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