dhcpcd-discuss

On 2019-07-12 16:08, Roy Marples wrote:

On 12/07/2019 14:48, feedback@xxxxxxxxxxxxx wrote:

Hi Roy,

I've discovered a minor problem during execution of dhcpcd hook scripts.
I have 20-resolv.conf and 50-ntp.conf active and a generated ntp.conf file looks like this:

# Generated by dhcpcd from # Generated by dhcpcd from eth0.dhcp, eth0.dhcp6, eth0.ra, eth0.static, eth0.dhcp, eth0.ra, eth0.static
server XX.XX.XX.XX
server XX.XX.XX.XX
# End of dhcpcd from # Generated by dhcpcd from eth0.dhcp, eth0.dhcp6, eth0.ra, eth0.static, eth0.dhcp, eth0.ra, eth0.static

I tracked this down to the 'header' variable in build_ntp_conf() which is not reset/unset before use so it still has the value from build_resolv_conf() where the same variable name is used and all variables are by default global if not declared otherwise.

--- 50-ntp.conf-orig
+++ 50-ntp.conf
@@ -62,6 +62,7 @@
         # Build a list of interfaces
         interfaces=$(list_interfaces "$ntp_conf_dir")

+        header=
         servers=
         if [ -n "$interfaces" ]; then
                 # Build the header

--- 20-resolv.conf-orig 2019-07-12 15:23:55.294195018 +0200
+++ 20-resolv.conf      2019-07-12 15:23:31.810088572 +0200
@@ -19,6 +19,7 @@
         interfaces=$(list_interfaces "$resolv_conf_dir")

         # Build the resolv.conf
+        header=
         if [ -n "$interfaces" ]; then
                 # Build the header
                 for x in ${interfaces}; do

Maybe we should check every variable in every function and if it is only used within that function declare it 'local' or use 'declare' itself (without option for strings) which would have the same effect. But this is just my bash knowledge, don't know how compatible these keywords are in sh and other shells.

Nice catch!
Fixed here: https://roy.marples.name/cgit/dhcpcd.git/commit/?id=21b556fd350cf269437694848cc0d52ce404432f

local isn't in POSIX shell and sadly the OpenBSD shell doesn't support it. Only shell I know of that doesn't, but heh ho.

Thanks

Roy

Small side note:
Theoretically there is still an alternative by unsetting every 'local' variable at the end of each function before returning, but that becomes quite ugly if there are multiple return's. So every function would either be (re-)written in a compatible way or another way would be to unset each variable as early as possible if they are not needed anymore in that function, that could maybe reduce the ugliness even in case of multiple return's.