Larry Garfield wrote:
On Wednesday 14 October 2009 12:02:28 am andy baxter wrote:
What I really want to know is, if I find the (relatively few) places in the code where php5.3 is causing an issue that actually breaks site functions, and apply the fix above (i.e. &$variable -> $variable) or something similar, is this a good way of solving the problem for the time being, given that the site goes public on thursday, or is it just going to break something else, or cause other problems like security holes or whatever?
It seems to have stopped the error in the couple of places I've already tried it, but it worries me to just convert a pass-by-reference into a straight parameter pass like this. I like to understand what I am doing rather than just apply a patch, which is why I was asking if anyone can explain a bit more about what's going on here?
cheers,
andy
What functions are we talking about? The answer to that will vary per the exact function being called.
Vis, show us da codez!
OK, as an example, this is the log entry for one of the errors that is coming up:
Type php Date Tuesday, 13 October, 2009 - 18:10 User experiment http://www.lancs.ac.uk/fass/drupal/expt/users/experiment Location http://www.lancs.ac.uk/fass/drupal/expt/profile/experimentalist/edit Referrer http://www.lancs.ac.uk/fass/drupal/expt/profile/experimentalist Message /Parameter 1 to theme_imagefield_widget() expected to be a reference, value given/ in //export/depts/fass/drupal/expt/includes/theme.inc/ on line /617/. Severity error Hostname 83.216.128.236 Operations
This is one of the errors that actually breaks site functions - the upload image box doesn't come up on the edit profile page.
I have grepped for the function name and found this: andy@anthill:/local/www/experiment$ grep -R 'function.*theme_imagefield_widget' * sites/all/modules/imagefield/imagefield.module:function theme_imagefield_widget_preview($item = NULL) { sites/all/modules/imagefield/imagefield.module:function theme_imagefield_widget_item($element) { sites/all/modules/imagefield/imagefield_widget.inc:function theme_imagefield_widget(&$element) {
I assume that the last declaration is the relevant one because of the '&$element'. This function looks like this:
/** * FormAPI theme function. Theme the output of an image field. */ function theme_imagefield_widget(&$element) { drupal_add_css(drupal_get_path('module', 'imagefield') .'/imagefield.css'); return theme('form_element', $element, $element['#children']); }
Here is another one, which doesn't break anything serious as far as I can tell:
Type php Date Tuesday, 13 October, 2009 - 23:28 User experiment http://www.lancs.ac.uk/fass/drupal/expt/users/experiment Location http://www.lancs.ac.uk/fass/drupal/expt/blog/previously-test/edit Referrer http://www.lancs.ac.uk/fass/drupal/expt/blog/previously-test/edit Message /Parameter 1 to content_diff() expected to be a reference, value given/ in //export/depts/fass/drupal/expt/includes/module.inc/ on line /471/. Severity error Hostname 83.216.128.236 Operations
Same procedure: andy@anthill:/local/www/experiment$ grep -R 'function.*content_diff' * sites/all/modules/cck/includes/content.diff.inc:function content_diff(&$old_node, &$new_node) { sites/all/modules/cck/includes/content.diff.inc: $function = $field_types[$field['type']]['module'] . '_content_diff_values'; sites/all/modules/cck/includes/content.diff.inc: $function = function_exists($function) ? $function : 'content_content_diff_values'; sites/all/modules/cck/includes/content.diff.inc:function content_content_diff_values($node, $field, $items) { sites/all/modules/cck/includes/content.diff.inc: function userreference_content_diff_values($node, $field, $items) { sites/all/modules/cck/includes/content.diff.inc: function nodereference_content_diff_values($node, $field, $items) { sites/all/modules/cck/content.module: if (module_exists('diff') && !function_exists('content_diff')) {
Here it looks like the first line above; the function looks like this: /** * Implementation of hook_diff() */ function content_diff(&$old_node, &$new_node) { $result = array(); // Prevent against invalid 'nodes' built by broken 3rd party code. if (isset($new_node->type)) { $type = content_types($new_node->type); $field_types = _content_field_types(); foreach ($type['fields'] as $field) { // Ignore fields the current user is not allowed to view. if (!content_access('view', $field, NULL, $new_node)) { continue; } $function = $field_types[$field['type']]['module'] . '_content_diff_values '; $function = function_exists($function) ? $function : 'content_content_diff _values'; $old_values = array(); $new_values = array(); if (isset($old_node->$field['field_name'])) { $old_values = $function($old_node, $field, $old_node->$field['field_name ']); } if (isset($new_node->$field['field_name'])) { $new_values = $function($new_node, $field, $new_node->$field['field_name']); } if ($old_values || $new_values) { $result[$field['field_name']] = array( '#name' => $field['widget']['label'], '#old' => $old_values, '#new' => $new_values, '#weight' => $field['widget']['weight'], '#format' => array( 'show_header' => FALSE, ), ); } } } return $result; }
I could just remove the & in front of the parameter name in both cases, but doing that without understanding what I am doing makes me nervous. Why would another function be passing a variable by value when the function expects a reference? Is this just bad coding which PHP <5.3 used to cover for, or some older feature which has been removed?
You could tell me in each case whether you think this hack is safe to use, or suggest another one, but what I really need is some better understanding of what's happening and why this situation has come about so I can do it myself - there are over 400 files in my installation which include functions with variable declarations like this, so presumably I can't just change all of them from &$ to $.
andy