bsdperimeter/pfsense — pull request #206 by phil-davis (2012-08-06 19:18:15+00:00)
Title: change $rcfileprefix to $g['local_rc_path']

See forum report http://forum.pfsense.org/index.php/topic,52280.0.html
The problem with $rcfileprefix defined in service-utils.inc is that if there is a require_once("service_utils.inc") inside a function then the variable $rcfileprefix is a local variable in that function, not a global. But if the require_once("service_utils.inc") is outside of a function then $rcfileprefix becomes global.
Declaring variables in the hope that they will be included in an appropriate place and become global can go wrong, like here, different calling sequences with different ways of including the files can end up with the variable being global or local (or both?), resulting in confusion about how to reference it.
$rcfileprefix is used as a path so it seems appropriate to put it in $g along with the other paths already there.
The code already organises for $g to be globally defined.
I think this is a good solution for this case - but please have a look and see what you think is the best coding standard for these sort of "constants".
I have tried installing packages, stopping, starting services, rebooting, removing services. The code is working in all these various cases.
Shutdown now outputs:
pfSense is rebooting now.
Stopping package pfBlocker...done.
Stopping package squid3...done.
Stopping package darkstat...done.
Stopping /usr/local/etc/rc.d/proxy_monitor.sh...done.
Stopping /usr/local/etc/rc.d/squid.sh...done.
Syntax error: Bad fd number

Note1: on a separate issue, packages like "squid3" actually have a stop/start script called squid.sh and also has proxy_monitor.sh. The code stops the package "squid3" then it finds "squid.sh" that it thinks is not accounted for, so it calls that also (which does no harm). It would be good to have a way that the code could know that a package (like "squid3") really has a basename "squid", in order to match things like squid.sh - there doesn't seem to be a way for the code to reliably make a match between the package "squid3" and the service "squid" with its rc file "squid.sh".
Note2: It also calls proxy_monitor.sh "stop", but proxy_monitor.sh is not written to take any parameters, it doesn't have the expected stop/start/restart format. Perhaps proxy_monitor.sh should have code to allow it to be called with "stop" and "restart"?
Note3: In other cases it might not be appropriate to add other globals to $g. Maybe it would often be better to make them constants with define(). Constants are always global, wherever they are declared. There are a couple of limitations - array constants are not allowed, constants can't be substituted in some heredoc and string formats. The other option is to make a function that returns the constant/s - anything that needs the value can call the function. Perhaps the development team might like to discuss a policy for this and then the use of odd globals can be removed from the code. (e.g. get down to having just $g and $config as the global variables that are global across lots of modules.)