Problem/Motivation
There are many issue reports of receiving a message of the form
Deprecated function: Call-time pass-by-reference has been deprecated in function_name() (line nnnn of sourcefile).
... but the message does not come from the source line mentioned. The message is produced when the PHP is parsed during require/include(_once) processing.
In _drupal_error_handler_real() the call to _drupal_log_error() reports the source line and file information from _drupal_get_last_caller().
So we get the message:
Deprecated function: Call-time pass-by-reference has been deprecated in oik_settings_form() (line 161 of C:\...\sites\all\modules\_custom\oik\oik.module).
but the code at line 161 was
require_once( "bobbformd.inc" );
The actual clue to the problem is passed in the data returned from debug_backtrace() for drupal_error_handler (and drupal_error_handler).
C:\...\includes\bootstrap.inc(2207:0) _drupal_error_handler_real 3 Array ( [0] => 8192 [1] => Call-time pass-by-reference has been deprecated [2] => C:\...\sites\all\modules\_custom\oik\bobbformd.inc [3] => 122 [4] => Array ( ) ) C:\...\sites\all\modules\_custom\oik\oik.module(161:0) _drupal_error_handler 4 Array ( [0] => 8192 [1] => Call-time pass-by-reference has been deprecated [2] => C:\...\sites\all\modules\_custom\oik\bobbformd.inc [3] => 122 [4] => Array ( ) ) C:\...\sites\all\modules\_custom\oik\oik.module(161:0) oik_settings_form 5 (0:0) oik_settings_form 6 Array ( [0] => Array (
where line 122 of bobbformd.inc was
function actpbrc( &$byref) { actpbra( &$byref ); }
The "error" being the & on the call to actpbra().
So to aid problem determination we need to provide a bit more information from the debug_backtrace data in the error log.
Proposed resolution
Rather than change _drupal_get_last_caller() I realised that the "missing" information was being passed to _drupal_error_handler_real() in the parameters. So a simple fix is to issue TWO messages when the $filename and $line passed don't match $caller['file'] or $caller['line'] returned from _drupal_get_last_caller().
Add this code before the current call to _drupal_log_error().
if ( $caller['file'] != $filename || $caller['line'] != $line ) {
_drupal_log_error(array(
'%type' => isset($types[$error_level]) ? $severity_msg : 'Unknown error',
// The standard PHP error handler considers that the error messages
// are HTML. We mimick this behavior here.
'!message' => filter_xss_admin($message),
'%function' => "unknown",
'%file' => $filename,
'%line' => $line,
'severity_level' => $severity_level,
), $error_level == E_RECOVERABLE_ERROR);
}
The first message shows where the error actually occurred and the second shows most recent function call.
Deprecated function: Call-time pass-by-reference has been deprecated in unknown (line 122 of C:\...\sites\all\modules\_custom\oik\bobbformd.inc). Deprecated function: Call-time pass-by-reference has been deprecated in oik_settings_form() (line 161 of C:\...\sites\all\modules\_custom\oik\oik.module).
Remaining tasks
It would probably be nicer to make it clear that the additional information provided by the second message is to help locate the actual message.
User interface changes
N/A?
API changes
None
Comments
Comment #1
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedthe file to patch is includes/error.inc
and this works soooo very well.
Thank you.
Because Drupal 7 now makes recoverable errors, fatal, I could not run crush. this is a great addition.
Comment #2
Fabianx CreditAttribution: Fabianx commentedGreat patch!
Does the job ...
Comment #3
SocialNicheGuru CreditAttribution: SocialNicheGuru commentedhow can we get this into core?
Comment #4
oriol_e9gYou need to create a patch
Comment #5
marcingy CreditAttribution: marcingy commentedAnd it needs to start life in d8.
Comment #6
Fabianx CreditAttribution: Fabianx commentedI just stumbled upon that again.
Here is a first patch for that in 8.x.
Edit:
Yes! - It also applies to 7.x still. Fun!
Comment #7
pingers CreditAttribution: pingers commentedMissing period. "Catch errors created via require and drupal_load." is better I think.
Created a test module with a call to a function using a reference:
Before patch:
Deprecated function: Call-time pass-by-reference has been deprecated in drupal_load() (line 1157 of /var/www/drupal/core/includes/bootstrap.inc).
After patch:
Deprecated function: Call-time pass-by-reference has been deprecated in unknown (line 6 of /var/www/drupal/sites/all/modules/test/test.module).
RTBC if green. I don't think a minor string change precludes me from rtbc'ing :)
Comment #8
Fabianx CreditAttribution: Fabianx commentedUltimately this needs a backport to D7.
Comment #9
pingers CreditAttribution: pingers commentedI'm a dummy. Function names (require and drupal_load) need (). Re-rolled.
Comment #10
Fabianx CreditAttribution: Fabianx commentedxpost
Comment #11
webchickChecking. Is it possible to add test coverage for this?
Comment #12
Fabianx CreditAttribution: Fabianx commentedComment #15
mgiffordThis still a concern in D8? Unassigned issue too.
Comment #16
Fabianx CreditAttribution: Fabianx at Tag1 Consulting commentedYes, that would still be useful for D8.
Comment #29
quietone CreditAttribution: quietone at PreviousNext commentedThere has been no activity here for 8 years. The function in the patch was removed from core in 2011, 12 years ago. I am not able to figure what might still need to be done.
Is there anything still to do here?
I am setting the status to Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #30
Kristen PolAs part of the Bug Smash Initiative, we are triaging issues that are marked "Postponed (maintainer needs more info)". This issue was marked "Postponed (maintainer needs more info)" 3 months ago.
Since we need additional information for this issue to progress, I'm marking the issue "Closed (cannot reproduce)". If anyone can provide complete steps to reproduce the issue (starting from "Install Drupal core"), document those steps in the issue summary and set the issue status back to "Active" [or "Needs Work" if it has a patch, etc.].
Thanks!