Hi PHPBB!
First thanks for doing a great job on making an awesome piece of software.
I'm in the process of integrating authentication from a web site into PHPBB3 and during this process I have read quite a few lines of the PHP code. One thing that struck me is that include(…) are preferred over include_once in many places (274 places to be exact). Is there any particular reason that include_once isn't used instead?
One example is on line 862 in /posting.php:
include($phpbb_root_path . 'includes/functions_user.' . $phpEx);
where the user functions are included. This might cause a "Cannot redeclare ..." error as includes/functions_user.php defines some very useful functions, that might have been included previously (eg. in an authentication file, like auth_ldap.php).
Anyways what I'm asking is basically whether it would be safe for me to change all include(…) to
include_once(….)?
Thanks again for a nice piece of software.
Best regards
Jakob
Why is include favored over include_once?
- imkingdavid
- Registered User
- Posts: 1050
- Joined: Thu Jul 30, 2009 12:06 pm
Re: Why is include favored over include_once?
IIRC, include_once() creates unnecessary overhead that can be avoided by instead using something like:
Where function_exists() could also be class_exists() or defined() depending on how you are determining whether the file has been included.
As for your question
EDIT: See kellanved's (he's a former development team member) answer in this topic.
Code: Select all
if (!function_exists('some_fairly_common_function'))
{
include('path/to/file');
}
As for your question
The core code already takes into account what files have been included, so this isn't an issue (if there is some question, we would use some method like I suggested above). For code modifications by users, it is a good idea to by default do what I suggested above just to be safe (especially if two MODs try to include the same file).jakobgt wrote:This might cause a "Cannot redeclare ..." error as includes/functions_user.php defines some very useful functions, that might have been included previously (eg. in an authentication file, like auth_ldap.php).
EDIT: See kellanved's (he's a former development team member) answer in this topic.
Re: Why is include favored over include_once?
If unguarded includes create a problem, we should probably change them to guarded includes.jakobgt wrote: One example is on line 862 in /posting.php:
include($phpbb_root_path . 'includes/functions_user.' . $phpEx);
where the user functions are included. This might cause a "Cannot redeclare ..." error as includes/functions_user.php defines some very useful functions, that might have been included previously (eg. in an authentication file, like auth_ldap.php).
Re: Why is include favored over include_once?
The function/class that is used should be checked for existence.imkingdavid wrote:Code: Select all
if (!function_exists('some_fairly_common_function'))
- imkingdavid
- Registered User
- Posts: 1050
- Joined: Thu Jul 30, 2009 12:06 pm
Re: Why is include favored over include_once?
Yes, I wasn't saying to just use any function from the file, but I didn't have a specific example.Oleg wrote:The function/class that is used should be checked for existence.
Re: Why is include favored over include_once?
Thanks for the quick reply!
Secondly many of the acp_...php files in includes/acp does not guard the inclusion of includes/functions_user.php and hence if one was to include this file logically in the top of the file in an auth_... file, then one gets an "Cannot redeclare .." error.
I agree that using include_once (and require_once) induces a speed penalty over the include (and require) counterparts (especially in PHP4). But as this post http://stackoverflow.com/a/194959 points out is not the case anymore in PHP5 (remember also that you have the overhead of function_exists in the guarded version). So my guess is that you still want to support PHP4 by guarding all of the includes, but is that really still necessary? I mean, how many PHP4 installations exists "out there on the wild wild Internet"? And how many are used for PHPBB?imkingdavid wrote:IIRC, include_once() creates unnecessary overhead that can be avoided by instead using something like:Code: Select all
if (!function_exists('some_fairly_common_function')) { include('path/to/file'); }
Secondly many of the acp_...php files in includes/acp does not guard the inclusion of includes/functions_user.php and hence if one was to include this file logically in the top of the file in an auth_... file, then one gets an "Cannot redeclare .." error.
- imkingdavid
- Registered User
- Posts: 1050
- Joined: Thu Jul 30, 2009 12:06 pm
Re: Why is include favored over include_once?
From what I can remember, when phpBB 3.0 was released (iirc around late 2007 or early 2008), even though PHP 5 was released, the vast majority of shared hosts (and therefore the vast majority of phpBB users) were still on PHP 4. Since then, yes most have upgraded, but we have not yet updated the code.jakobgt wrote:I agree that using include_once (and require_once) induces a speed penalty over the include (and require) counterparts (especially in PHP4). But as this post http://stackoverflow.com/a/194959 points out is not the case anymore in PHP5 (remember also that you have the overhead of function_exists in the guarded version). So my guess is that you still want to support PHP4 by guarding all of the includes, but is that really still necessary? I mean, how many PHP4 installations exists "out there on the wild wild Internet"? And how many are used for PHPBB?
I agree that that is problematic, if code before a file is included (unguarded in the core) needs to use functions from that file. But I don't know enough about it the difference in overhead between PHP 4 and 5 or between the *_once functions and the *_exists() functions to suggest that we change to the *_once variants.jakobgt wrote:Secondly many of the acp_...php files in includes/acp does not guard the inclusion of includes/functions_user.php and hence if one was to include this file logically in the top of the file in an auth_... file, then one gets an "Cannot redeclare .." error.
And until that, if you need to use a function in an include file that is included (unguarded in the core) after you need it, I guess you could submit a bug report asking the include to be changed to a guarded included for that reason.
Re: Why is include favored over include_once?
Okay, no worries. I'll just submit a bug report about it. I'll see if I can find as many places of the unguarded include as possible.imkingdavid wrote: And until that, if you need to use a function in an include file that is included (unguarded in the core) after you need it, I guess you could submit a bug report asking the include to be changed to a guarded included for that reason.
Thanks.
- A_Jelly_Doughnut
- Registered User
- Posts: 1780
- Joined: Wed Jun 04, 2003 4:23 pm
Re: Why is include favored over include_once?
Its irrelevant anyway, because 3.1 and higher require PHP 5.2, and any change of this magnitude would likely not go in the stable branch.jakobgt wrote: I agree that using include_once (and require_once) induces a speed penalty over the include (and require) counterparts (especially in PHP4). But as this post http://stackoverflow.com/a/194959 points out is not the case anymore in PHP5 (remember also that you have the overhead of function_exists in the guarded version). So my guess is that you still want to support PHP4 by guarding all of the includes, but is that really still necessary? I mean, how many PHP4 installations exists "out there on the wild wild Internet"? And how many are used for PHPBB?
Might be worth discussing moving to include_once/require_once in the appropriate forum. (3.1 or 3,2 discussion_
A_Jelly_Doughnut
- bantu
- 3.0 Release Manager
- Posts: 557
- Joined: Thu Sep 07, 2006 11:22 am
- Location: Karlsruhe, Germany
- Contact:
Re: Why is include favored over include_once?
We can change unguarded includes to guarded includes in phpBB 3.0.x if absolutely necessary. In 3.1.x we have autoloading anyway, so new code will probably be autoloaded instead of manually loaded.