Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 528314 - net-misc/netifrc-0.2.2 with /bin/sh -> dash - /lib64/rc/sh/ 106: local: :: bad variable name
Summary: net-misc/netifrc-0.2.2 with /bin/sh -> dash - /lib64/rc/sh/ 106:...
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: netifrc Team
Depends on: 780573
Blocks: nonbash
  Show dependency tree
Reported: 2014-11-05 12:59 UTC by OGINO Masanori
Modified: 2021-04-15 22:07 UTC (History)
3 users (show)

See Also:
Package list:
Runtime testing required: ---

/etc/conf.d/net with 6to4 settings (file_528314.txt,153 bytes, text/plain)
2014-11-05 13:07 UTC, OGINO Masanori
The output of `/etc/init.d/net.6to4 --debug start` (6to4.log,47.46 KB, text/plain)
2016-12-17 23:53 UTC, OGINO Masanori
The output of `/etc/init.d/net.6to4 --debug start`, with the patch suggested by robbat2 (6to4.log,54.69 KB, text/plain)
2016-12-18 01:58 UTC, OGINO Masanori

Note You need to log in before you can comment on or make changes to this bug.
Description OGINO Masanori 2014-11-05 12:59:39 UTC
I fails to create 6to4 tunnels if /bin/sh points to dash. I succeeds it with the same configuration if /bin/sh points to bash.

Error messages:
>  * Bringing up interface 6to4
>  *   Creating 6to4 tunnel on 6to4 ...                                   [ ok ]
>  *   ip6to4 ...
> /lib64/rc/sh/ 106: local: :: bad variable name
>  * ERROR: net.6to4 failed to start

Reproducible: Always

Steps to Reproduce:
1. Make /bin/sh point to dash e.g. with eselect sh.
2. Edit /etc/conf.d/net to enable a 6to4 interface named ${6TO4}.
3. cd /etc/init.d; ln -s net.lo net.${6TO4}
4. /etc/init.d/net.${6TO4} start
Actual Results:  
net.${6TO4} fails to start.

Expected Results:  
net.${6TO4} starts successfully.

Portage 2.2.8-r2 (default/linux/amd64/13.0, gcc-4.8.3, glibc-2.19-r1, 3.16.5-gentoo x86_64)
System uname: Linux-3.16.5-gentoo-x86_64-Intel-R-_Celeron-R-_CPU_3.06GHz-with-gentoo-2.2
KiB Mem:     3976392 total,   3870168 free
KiB Swap:    3905532 total,   3905532 free
Timestamp of tree: Wed, 05 Nov 2014 04:45:01 +0000
ld GNU ld (Gentoo 2.24 p1.4) 2.24
app-shells/bash:          4.2_p53
dev-lang/perl:            5.18.2-r2
dev-lang/python:          2.7.7, 3.3.5-r1, 3.4.1
dev-util/pkgconfig:       0.28-r1
sys-apps/baselayout:      2.2
sys-apps/openrc:          0.12.4
sys-apps/sandbox:         2.6-r1
sys-devel/autoconf:       2.69
sys-devel/automake:       1.13.4
sys-devel/binutils:       2.24-r3
sys-devel/gcc:            4.8.3
sys-devel/gcc-config:     1.7.3
sys-devel/libtool:        2.4.2-r1
sys-devel/make:           4.0-r1
sys-kernel/linux-headers: 3.13 (virtual/os-headers)
sys-libs/glibc:           2.19-r1
Repositories: gentoo
CFLAGS="-O2 -pipe"
CONFIG_PROTECT_MASK="/etc/ca-certificates.conf /etc/env.d /etc/gconf /etc/gentoo-release /etc/revdep-rebuild /etc/sandbox.d /etc/terminfo"
CXXFLAGS="-O2 -pipe"
FCFLAGS="-O2 -pipe"
FEATURES="assume-digests binpkg-logs config-protect-if-modified distlocks ebuild-locks fixlafiles merge-sync news parallel-fetch preserve-libs protect-owned sandbox sfperms strict unknown-features-warn unmerge-logs unmerge-orphans userfetch userpriv usersandbox usersync"
FFLAGS="-O2 -pipe"
LDFLAGS="-Wl,-O1 -Wl,--as-needed"
PORTAGE_RSYNC_OPTS="--recursive --links --safe-links --perms --times --omit-dir-times --compress --force --whole-file --delete --stats --human-readable --timeout=180 --exclude=/distfiles --exclude=/local --exclude=/packages"
USE="acl amd64 berkdb bindist bzip2 cli cracklib crypt cxx dri emacs fortran gdbm iconv ipv6 mmx modules multilib ncurses nls nptl openmp pam pcre readline session sse sse2 ssl tcpd unicode vim-syntax zlib" ABI_X86="64" ALSA_CARDS="ali5451 als4000 atiixp atiixp-modem bt87x ca0106 cmipci emu10k1x ens1370 ens1371 es1938 es1968 fm801 hda-intel intel8x0 intel8x0m maestro3 trident usb-audio via82xx via82xx-modem ymfpci" APACHE2_MODULES="authn_core authz_core socache_shmcb unixd actions alias auth_basic authn_alias authn_anon authn_dbm authn_default authn_file authz_dbm authz_default authz_groupfile authz_host authz_owner authz_user autoindex cache cgi cgid dav dav_fs dav_lock deflate dir disk_cache env expires ext_filter file_cache filter headers include info log_config logio mem_cache mime mime_magic negotiation rewrite setenvif speling status unique_id userdir usertrack vhost_alias" CALLIGRA_FEATURES="kexi words flow plan sheets stage tables krita karbon braindump author" CAMERAS="ptp2" COLLECTD_PLUGINS="df interface irq load memory rrdtool swap syslog" ELIBC="glibc" GPSD_PROTOCOLS="ashtech aivdm earthmate evermore fv18 garmin garmintxt gpsclock itrax mtk3301 nmea ntrip navcom oceanserver oldstyle oncore rtcm104v2 rtcm104v3 sirf superstar2 timing tsip tripmate tnt ublox ubx" INPUT_DEVICES="keyboard mouse evdev" KERNEL="linux" LCD_DEVICES="bayrad cfontz cfontz633 glk hd44780 lb216 lcdm001 mtxorb ncurses text" LIBREOFFICE_EXTENSIONS="presenter-console presenter-minimizer" OFFICE_IMPLEMENTATION="libreoffice" PHP_TARGETS="php5-5" PYTHON_SINGLE_TARGET="python2_7" PYTHON_TARGETS="python2_7 python3_3" RUBY_TARGETS="ruby19 ruby20" USERLAND="GNU" VIDEO_CARDS="fbdev glint intel mach64 mga nouveau nv r128 radeon savage sis tdfx trident vesa via vmware dummy v4l" XTABLES_ADDONS="quota2 psd pknock lscan length2 ipv4options ipset ipp2p iface geoip fuzzy condition tee tarpit sysrq steal rawnat logmark ipmark dhcpmac delude chaos account"
Comment 1 OGINO Masanori 2014-11-05 13:07:06 UTC
Created attachment 388582 [details]
/etc/conf.d/net with 6to4 settings

This is copied from my configuration file, but removed unnecessary details (I think) such as the content of config_enp3s0. I use static IP(v4) addresses in them.
Comment 2 SpanKY gentoo-dev 2015-05-25 10:07:22 UTC
if this is still happening, please run:
$ /etc/init.d/net.${6TO4} --debug start >& log

and then attach that log here for us
Comment 3 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2016-10-24 21:36:04 UTC
No response from user with needed debug output.
Comment 4 OGINO Masanori 2016-12-17 23:53:05 UTC
Created attachment 456542 [details]
The output of `/etc/init.d/net.6to4 --debug start`

I'm sorry, I've missed notifications until now.

The original machine has gone, but the problem remains on my current machine.
The log on the current machine is attached.

Would you reopen this bug?
Comment 5 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2016-12-18 01:13:24 UTC
Great to hear back from you!

Can you please check net/, and experiment with patching this, 
-local routes_ip6to4=$(service_get_value "routes_ip6to4_$IFVAR")
+local routes_ip6to4="$(service_get_value "routes_ip6to4_$IFVAR")"

If that does fix it, I need to confirm part of the POSIX SH spec on where quoting is required during assignment.
Comment 6 OGINO Masanori 2016-12-18 01:54:40 UTC
(In reply to Robin Johnson from comment #5)
> Great to hear back from you!
> Can you please check net/, and experiment with patching this, 
> -local routes_ip6to4=$(service_get_value "routes_ip6to4_$IFVAR")
> +local routes_ip6to4="$(service_get_value "routes_ip6to4_$IFVAR")"
> If that does fix it, I need to confirm part of the POSIX SH spec on where
> quoting is required during assignment.

After applying the patch, 6to4 service starts successfully and I confirmed a 6to4 interface up via `ifconfig`!
I will attach a log with the patch soon.
Comment 7 OGINO Masanori 2016-12-18 01:58:18 UTC
Created attachment 456546 [details]
The output of `/etc/init.d/net.6to4 --debug start`, with the patch suggested by robbat2

The patch put in /etc/portage/patches/net-misc/netifrc before `emerge -1 netifrc`:

diff -ru netifrc-0.5.1.orig/net/ netifrc-0.5.1/net/
--- netifrc-0.5.1.orig/net/    2016-12-18 10:39:37.570023202 +0900
+++ netifrc-0.5.1/net/ 2016-12-18 10:40:46.450023440 +0900
@@ -103,7 +103,7 @@
        local config_ip6to4=$(service_get_value "config_ip6to4_$IFVAR")
-       local routes_ip6to4=$(service_get_value "routes_ip6to4_$IFVAR")
+       local routes_ip6to4="$(service_get_value "routes_ip6to4_$IFVAR")"

        # Now apply our config
        eval config_${config_index}=\'"${config_ip6to4}"\'
Comment 8 Kerin Millar 2019-04-21 03:33:35 UTC
In situations where word splitting would not otherwise affect the manner in which an assignment is parsed, quoting is most definitely not required by POSIX. For example, these are all correct:

  var=$othervar       # identical to var="$othervar"
  var=$(some_command) # identical to var="$(some_command)"

So what's the problem? Unfortunately, the problem is the use of the local keyword. Don't expect POSIX to explain this because local is a standards transgression to begin with. It is not documented by POSIX and, as such, no POSIX-confirming shell is obligated to implement local in the first place. Any script that purports to target #!/bin/sh and proceeds to use local is non-portable, by definition.

That said, let's take a closer look at what's going on. Firstly, bash behaves exactly as we'd expect.

  $ code='f() { local a=$(printf %s "$1"); local b="$(printf %s "$1")"; printf "<%s>" "$a" "$b"; }; f "foo bar"'
  $ bash --posix -c "$code"
  <foo bar><foo bar>

So does busybox ash.

  $ busybox sh -c "$code"
  <foo bar><foo bar>

So does mksh, from the MirBSD project.

  $ mksh -c "$code"
  <foo bar><foo bar>

And so does loksh, a port of the excellent incarnation to be found in OpenBSD.

  $ ksh -c "$code"
  <foo bar><foo bar>

Unfortunately, things start to fall apart in other popular shells that may act as a valid /bin/sh.

  $ dash -c "$code"
  <foo><foo bar>

  $ posh -c "$code" # yes, even posh supports local
  <foo><foo bar>

I tried several other candidates, such as ksh93, but found that most did not provide a local keyword. No surprises there. Even though POSIX can offer no guidance here, I would consider the behaviour of dash to be a bug. Still, as things stand, we must deal with the consequences.

@William (and, indeed, any gentoo dev inclined to listen), if you are going to continue using local then, at the very least, I would recommend getting into the habit of not using local in conjunction with an assignment. That may sound unduly stringent but please keep in mind that local has other problems that can easily trap the unwary. Case in point:

  $ bash -c 'f() { local var=$(false) || exit; echo "still running"; }; f'
  $ bash -c 'f() {       var=$(false) || exit; echo "still running"; }; f'

One might reasonably assume that these are equivalent. However, one would be terribly wrong. Where using local, the exit status value of the command substitution is masked! Not only that, but dash inherits this design flaw. The simplest way to avoid all of these issues is to never declare a variable with local, while simultaneously assigning to it.
Comment 9 Kerin Millar 2019-04-21 03:46:26 UTC
Just to add that, whatever one may think of my recommendation, double-quoting the expansion of the command substitution is a valid way of working around the issue in dash and other shells that behave as dash does, on the condition that one fully understands - and accepts - the consequences of local squashing the exit status value to 0.
Comment 10 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2020-05-31 06:05:04 UTC
(In reply to Kerin Millar from comment #9)
> Just to add that, whatever one may think of my recommendation,
> double-quoting the expansion of the command substitution is a valid way of
> working around the issue in dash and other shells that behave as dash does,
> on the condition that one fully understands - and accepts - the consequences
> of local squashing the exit status value to 0.

I don't think there's any spots in the codebase that presently depend on the exit status of a subshell command there (I did a quick review only).

That said, part of this is never having a clear specification of exactly what the target dialect of Shell is.

There was a GSOC project that contributed the systemd hooks and that started some of the test harness requirements for this project, but they need a LOT of additional work to be usable.

It should ideally test with all different shells:
- bash
- busybox sh
- mksh
- dash
- posh

I don't think csh/ksh will be supported, they are just too far away from the rest in a few cases.
Comment 11 Robin Johnson archtester Gentoo Infrastructure gentoo-dev Security 2020-05-31 06:06:52 UTC
This is patched for now
Comment 12 Kerin Millar 2020-05-31 14:13:22 UTC
Thanks, Robin. I'm sure you're right but I'll check again also.

As an aside, there is actually a way to approximate the effect of local in POSIX sh, which is to use a different form of compound command to contain the function body.

my_func() (
    # This is a compound command that runs in a subshell.
    # The assignment to var will be undone upon finishing.

That works nicely, though it is obviously not suitable where intentionally manipulating variables that were defined in some other scope. Still, I tend to use this pattern in my own 'pure' sh code as of late.

I concur that some of the problems stem from not being clear as to the target shell, or shells as the case may be. Presently, the project ostensibly feels as though it wants to target POSIX sh in principle, while not being committed to that requirement. As I remarked elsewhere, I've reached the conclusion - after many years of practice - that, more often than not, it pays to target bash other than for the most trivial programs. I think that the lack of arrays, in particular, is damaging to netifrc and the overall quality of openrc runscripts. One ends up outright relying on the unfortunate behaviour of word splitting (with set -f sadly nowhere to be seen), questionable eval usage and so on. For splitting strings pretending to be arrays, even just having read -ra would be useful. That's not to mention the various other features of bash that would make it easier to solve relevant problems tidily, without forking off and running external utilities which have their own issues of portability.

All that said, the list of shells that you cited looks entirely reasonable, as concerns Gentoo's immediate needs.
Comment 13 Kerin Millar 2020-05-31 14:15:32 UTC
Actually, with one exception. I think that posh is useful for exploring certain portability concerns but that it's a waste of time specifically targeting it. I explain this view further in bug 720996.
Comment 14 Kerin Millar 2021-01-18 18:21:40 UTC
This has been open for a very long time now. Could the fix please be patched into 0.7.1 so that a working version with stable keywords is available? The bug could then be closed.
Comment 15 Kerin Millar 2021-04-15 20:23:31 UTC
Resolved by bug 780573.