PHP 5.3 doesn't allow us to pass array() for $form_state, because modules are expecting a reference for the $form_state parameter. We have to create a temporary variable and pass that instead.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cha0s’s picture

FileSize
1.05 KB

Apologies; that was not the correct fix. I neglected to use Drupal 6's mechanism to get the references in correctly. Here is an updated patch. I opted to directly pass in the actual $form_state, instead of an empty array (Was there a reason not to..?)

davidwatson’s picture

Patch runs like a dream on PHP 5.3 (CentOS 5.3), and clears up the associated "reference expected" warnings as well without any ill side-effects.

Thanks for contributing this! :] Could anyone confirm so we can RTBC?

EDIT: Patch in #1, if there was any confusion.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community

Indeed. That's exactly how drupal_prepare_form() does it:

  // __drupal_alter_by_ref is unset in the drupal_alter() function, we need
  // to repopulate it to ensure both calls get the data.
  $data['__drupal_alter_by_ref'] = array(&$form_state);
  drupal_alter('form', $data, $form_id);
b3liev3’s picture

Wow, it's great has fixed most of all my problems with php 5.3 warnings!

peternickson’s picture

Can someone please explain how to apply the patch to someone who's unfamiliar with php?

cha0s’s picture

@peternickson: Go to the base of your Drupal install, and run this:

wget -O - http://drupal.org/files/issues/php-5.3_0.patch | patch -p0

lukus’s picture

Thanks for the patch, it's cleared up my problems too.

@peternickson;

I ended up downloading the patch to the relevant 'cck/includes' directory .. then I ran the command 'patch < php-5.3_0.patch'.

cha0s’s picture

Oops yeah, you'll want to go to your CCK directory and apply the patch, not the Drupal base. Sorry about that!

Stupidscript’s picture

Fedora 12
PHP 5.3.2
Drupal 6.6.16

The patch linked to in comment #1 was unsuccessful in correcting my condition:

After navigating to the CCK module directory and executing the 'wget...' command shown in #6 I received:

patching file includes/content.node_form.inc
Hunk #1 succeeded at 465 (offset 143 lines).

So it would seem that the patch was successfully applied, but there is no change in the errors I see when a file is uploaded.

Modding common.inc to include the suggestion in http://drupal.org/node/710892 simply broke things ... for example on the Create Story node, it removed the option to attach a file and a custom list of categories was also missing until I removed the mod and everything went back to normal.

I ended up using the suggestion found at http://drupal.org/node/525856, and that either worked or suppressed the errors ... same result, in any case:

was: call_user_func_array($function, $args);
is: return call_user_func_array($function, &$args);

Just passing it along ...

[edit]
D'oh! Unless you want to go through a bunch of .inc files making similar changes, this should not be considered to be a fix. In a different node, uploading a file generated a similar error:

warning: Parameter 3 to bitcache_bitcache() expected to be a reference, value given in ../includes/module.inc on line 483.

So whatever is generating this error, it ain't fixed, yet. At least not by using one of the available suggestions.
[/edit]

cha0s’s picture

@Stupidscript the errors you are receiving are from different modules. This is not the right place to report errors from the bitcache module...

Stupidscript’s picture

@cha0s: Thank you. I was noting that I was/am continuing to experience the passed reference issue in multiple places within my Drupal 6 installation. The log entry for bitcache_bitcache() was just one of many instances. My log files are littered with hundreds of passed reference issues.

Bottom line: Neither of the patches or fixes offered in this thread nor the two I linked to in my post solved the passed reference problem for me.

Sorry for the confusion.

Is there a place I can go to learn why there are so many of these issues? What is the root cause? I tried your patch ... was that a simple fix for just one type of passed reference issue, or was it intended to fix all such issues? From the looks of it, it was intended to fix the issue every time drupal_alter() is called. That seems like an attempt at a global fix, so I am reporting that it did not act as a global fix, for my installation.

I see that the patch was successfully applied to modules/cck/includes/content.node_form.inc lines 468-470, so it's in there, but still receiving the errors ... including for modules like Bitcache, among many.

Thoughts? I sure would appreciate any.

KarenS’s picture

Status: Reviewed & tested by the community » Needs work

This patch changes the number of parameters passed to drupal_alter, that seems like it will break other things. I'm not comfortable committing this without someone explaining how that could be right.

Also the source of these problems is to prevent errors in PHP4, so it would help to have someone confirm that it doesn't break anything in PHP4.

fenstrat’s picture

Status: Needs work » Reviewed & tested by the community

This patch prevents php warnings in php 5.3 not php 4.

As Damien stated in #4, it's also exactly how drupal_prepare_form() does it.

TwoD’s picture

I got several of these "expected to be a reference" warnings in various modules as well, researched drupal_alter() a bit and came to the same conclusion, imitating what drupal_prepare_form() does is the right thing - and the #1 patch works beautifully!

I don't have PHP 4 installed locally at the moment to test the patch there, but considering Core works in 4 and 5 when doing things this way, CCK should too. I'm using this patch on all my PHP5 sites and agree with the RTBC status. Would love to see this committed. ;)

herveh’s picture

The patch didn't work for me
For now I'll take this temporary solution which worked for me :
http://drupal.org/node/710892#comment-2836432

hedac’s picture

I am still a bit lost with all these php 5.3 errors of Parameter x to xxxxxxxx_form_alter() expected to be a reference

is there a page / patch with a solution for all modules? it's not only a cck issue.

gapa’s picture

Any solution regarding this? I have few production sites with this problem now.

br,
gapa

fenstrat’s picture

The patch here in #1 is RTBC and applies to CCK.

@hedac @gapa For other PHP 5.3 issues there are solutions for the (shrinking) number of modules with outstanding issues. There's no one "page/patch" which solves all module's issues. You could try searching for the "PHP 5.3" tag, but any module with issues will need testing/fixing separately.

Fidelix’s picture

I can confirm that the patch at #1 fixes my issues.
I'm using cck 3.x-dev

nerdacus’s picture

I also confirm that the patch at #1 fixes my issues. I manually applied it just to be safe and no more error messages. Thank you very much. ...using cck 6.x-2.8...

roderik’s picture

@KarenS/#12, for completeness:

This patch changes the number of parameters passed to drupal_alter, that seems like it will break other things. I'm not comfortable committing this without someone explaining how that could be right.

drupal_alter() is just a wrapper around call_user_func_array(). As we can deduce from the code, the number of parameters passed to call_user_func_array() will still be the same as before the change. No difference, no breakage.

We can also deduce from the code, that drupal_alter() has actually been explicitly coded to be called like the patch in comment #1 does it. Hence: +1 for RTBC.

Renee S’s picture

Confirmed, fixed my problem on CCK "add more" button callbacks. CCK 2.x-dev, PHP5.3.

marioboro’s picture

Thank´s for the patch (#6). Worked for me perfectly!

jduhls’s picture

Thanks #1 and #5 thru #8...patch worked like a charm.

dasjo’s picture

subscribing

applying the patch didn't fix my problem as stated at http://drupal.org/node/919906#comment-3769680

roderik’s picture

@dasjo: since the patch in #1 really introduces no functional change, the response to the other is probably right: "your problem is in geocoder".

Just commenting to keep doubt from entering this issue again. This should really be applied & the issue closed.

rsevero’s picture

+1 for roderik comment above.

Also to reinforce, the explanation from roderick at #21 to KarenS's question from #12 is right on the mark.

The same issue happening at a different place might as well be solved here on this issue. I'm attaching another patch to solve the same issue in the upload module.

This patch should be applied together with the one from #1 as each one solves the same problem happening in two different places.

Please don't let this patch further delay the applying of patch from #1.

rsevero’s picture

And I forgot to attach the patch...

rsevero’s picture

I've just realized that this is a CCK issue. Sorry. Please disregard the patch at #28.

yched’s picture

FileSize
465 bytes

re @roderick :

drupal_alter() is just a wrapper around call_user_func_array(). As we can deduce from the code, the number of parameters passed to call_user_func_array() will still be the same as before the change. No difference, no breakage.

That code in drupal_alter() is pretty convoluted to track, so I did the following check :
- apply the attached patch for drupal_alter() - this displays the resulting $args param passed to call_user_func_array() (uses dsm(), thus requires devel.module)
- with current CCK-dev code, press 'add another item' on a multi-value field, and check the resulting array.
- Apply patch in #1 above, and press 'add another item' on a multi-value field : the resulting array is different.

So it does look like the change is not transparent ?

yched’s picture

Status: Reviewed & tested by the community » Needs work

Hm, the difference is precisely that $arg[1] now is the $form_state array(), instead of an empty array previsouly.

To be honest, the code in content_add_more_js() is an ugly convoluted mess which I cannot pretend I understand today - D7 made that much simpler, fortunately.
I don't remember the initial reasoning for calling drupal_alter() with an empty array instead of the $form_state array - which, at this point in the code, is a "fake", temporary form_state that gets ditched a couple lines below.

I don't think this would break too much stuff, but I'd feel more comfortable if the patch only fixed the 5.3 compatibility issue without changing existing behavior (behavior which may be right or wrong, but is unrelated to the issue at hand).

roderik’s picture

Assigned: cha0s » Unassigned
Status: Needs work » Needs review
FileSize
1.1 KB

Ok, that works. Don't change existing behavior.

(My #26 was wrong. I forgot that #1 does introduce a functional change, and the comment explicitly mentions that.)

Attached patch has been verified with the dsm() thing on a php5.2 & 5.3 installation.
Thanks for looking into this.

yched’s picture

Status: Needs review » Fixed

committed #32 to 2.x and 3.x branches

Thanks for the patience !

Anonymous’s picture

hi

my hosting upgraded to php 5.3 and i started getting errors for SecurePages_form_alter, i applied some fix i found to remove an "&" character and it solved that.

then i started getting the errors for the modules ( only when trying to use add_more in some content):

xmlsitemap_form_alter
xmlsitemap_node_form_alter
wysiwyg_form_alter
uc_restrict_qty_form_alter
uc_fee_form_alter
nodewords_form_alter
nodereference_url_form_alter
fivestar_form_alter
ddblock_form_alter

the patch in #1 fixed all my issues.

thanks

lucuhb’s picture

patch #1 solves also my warning pb when I click on "add more element" button. Thanks.

Status: Fixed » Closed (fixed)

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

ionmedia’s picture

i got warnings after push add more values
warning: Parameter 2 to date_form_alter() expected to be a reference and so on....

just remove & from function date_form_alter

function date_form_alter($form, $form_state, $form_id) {
if ($form_id == 'content_display_overview_form') {
date_content_display_form($form, $form_state);
}
}

because in php 5.3 we no need use & for reference

TwoD’s picture

@ionmedia, the real problem is in the code calling drupal_alter(), not in the date_form_alter() function signature.
The caller should use '__drupal_alter_by_ref' like in the patch from #32 (it has already been comitted to CCK so it will no longer be the cause of this problem, but other modules can still do it the wrong way).

chrisknight’s picture

I get an error:

[root@asdf cck]# wget -O - http://drupal.org/files/issues/php-5.3_0.patch | patch -p0
--2011-04-13 12:33:53-- http://drupal.org/files/issues/php-5.3_0.patch
Resolving drupal.org... 140.211.166.6, 140.211.166.21
Connecting to drupal.org|140.211.166.6|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 1073 (1.0K) [text/plain]
Saving to: `STDOUT'

100%[=====================================================================>] 1,073 --.-K/s in 0.004s

2011-04-13 12:33:53 (264 KB/s) - `-' saved [1073/1073]

patching file includes/content.node_form.inc
Hunk #1 FAILED at 322.
1 out of 1 hunk FAILED -- saving rejects to file includes/content.node_form.inc.rej
[root@asdf cck]#

Could someone just post the patched file? Thanks.

fenstrat’s picture

The patch in #32 was committed on 2010-12-06 and is part of cck-6.x-2.9 - there is no need to apply this patch.

milplus’s picture

apologies in advance for my ignorange on drupal

can I understand how I have to run this patch?

I do not know if I can run it using an application or how.

thanks

milplus’s picture

can you instruct me how i can apply this patch?

Fidelix’s picture

You don't have to.
You can just download the dev version of CCK, which already has the patch is incorporated in it.