Supercron, like many other Drupal 6.x modules, has php 5.3 compatibility issues. In this case, the function theme_supercron_settings() should accept a value, not a reference. Without it, the admin/settings/supercron does not display correctly.

Patch attached.

CommentFileSizeAuthor
#6 supercron.php53.patch764 bytespillarsdotnet
#3 theme_supercron_settings.patch510 bytespillarsdotnet
#2 Screen shot 2011-01-26 at 3.18.14 AM.png27.79 KBjohn franklin
supercron-php53.patch459 bytesjohn franklin

Comments

pillarsdotnet’s picture

pillarsdotnet commented
Status: Active » Needs review

Could you explain why an array should be passed by value, not by reference?

I know this is true for objects, but I was unaware that it is also true for arrays.

Can you point to some php 5.3 docs that explain this? Or at least describe the change in behavior your patch makes?

john franklin’s picture

john franklin commented
StatusFileSize
new Screen shot 2011-01-26 at 3.18.14 AM.png27.79 KB

Pre-patch: going to admin/settings/supercron displays a corrupted settings page (see image) and the error:

warning: Parameter 1 to theme_supercron_settings() expected to be a reference, value given in /usr/share/drupal6/includes/theme.inc on line 656.

Post-patch: the admin/settings/supercron page works as expected and no error is displayed.

I did the patch this way because (a) it doesn't change core and (b) it is similar to other php 5.3 patches I've seen.

pillarsdotnet’s picture

pillarsdotnet commented
Version: 6.x-1.4-beta1 » 6.x-2.x-dev
StatusFileSize
new theme_supercron_settings.patch510 bytes

Okay, so it's because that function gets called by call_user_func_array().

+1 on the patch, then.

Here's a re-roll against CVS HEAD.

cor3huis’s picture

cor3huis commented

Same for line 140
- function supercron_settings(&$form_state) {
to
+ function supercron_settings($form_state) {

I bet, just reviewing now

cor3huis’s picture

cor3huis commented
Title: PHP 5.3 Compatibility » Saving settings form not PHP 5.3 Compatible
Status: Needs review » Reviewed & tested by the community

Tested and it solves the issue at hand.

NOTE: manually also changed "supercron_settings(&$form_state) {" to "supercron_settings($form_state) {"
mind the &.

We hate &, it PHP not C ;)

pillarsdotnet’s picture

pillarsdotnet commented
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new supercron.php53.patch764 bytes

Confirmed that supercron_settings() also gets called via call_user_func_array().

Re-rolled patch.

cor3huis’s picture

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

This part of issues looks solved, tested via the supercron.combined.patch found here http://drupal.org/node/749606#comment-4013476

pillarsdotnet’s picture

pillarsdotnet commented

Abandoned module?

pillarsdotnet’s picture

pillarsdotnet commented
Status: Reviewed & tested by the community » Closed (duplicate)

Duplicate of #749606-16: Path to supercron.php incorrectly determined by the module code

Danzki’s picture

Danzki commented
Status: Closed (duplicate) » Reviewed & tested by the community

This patch solves my problem.