Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 850577

Summary: sys-apps/openrc-0.45 installs shell script that uses non-POSIX features
Product: Gentoo Linux Reporter: Agostino Sarubbo <ago>
Component: Current packagesAssignee: OpenRC Team <openrc>
Status: RESOLVED FIXED    
Severity: normal CC: kfm, sam
Priority: Normal    
Version: unspecified   
Hardware: All   
OS: Linux   
Whiteboard:
Package list:
Runtime testing required: ---
Bug Depends on:    
Bug Blocks: 609070    
Attachments: build.log

Description Agostino Sarubbo gentoo-dev 2022-06-08 06:43:52 UTC
https://blogs.gentoo.org/ago/2020/07/04/gentoo-tinderbox/

Issue: sys-apps/openrc-0.45 installs shell script that uses non-POSIX features.
Discovered on: amd64 (internal ref: ci)
Comment 1 Agostino Sarubbo gentoo-dev 2022-06-08 06:43:57 UTC
Created attachment 783533 [details]
build.log

build log and emerge --info
Comment 2 Kerin Millar 2022-06-08 07:34:28 UTC
This is an interesting case. Technically, it's a false positive. The expansion of HOSTNAME is the last of three methods that the script attempts to infer the hostname. That is why it is adorned with the following comment:

# checkbashisms: false positive (HOSTNAME var)

On the other hand, I have to ask what the value is in doing this to begin with. Here are the three methods in question:

1) reading from /etc/hostname
2) sourcing hostname from /etc/conf.d/hostname
3) expanding HOSTNAME in the hope that the shell set it

So, in the case that #3 applies, the shell is bash, something else will have set the hostname already and all the script will do is proceed to re-apply that hostname. What's the point? None whatsoever, as far as I can gather.

Further, the /etc/conf.d/hostname logic is flaky. It considers the fact that /etc/hostname contains at least one byte and is readable to mean that the hostname can be successfully collected that way but then doesn't bother to check the exit status of read, which is silly. A read is not guaranteed to succeed.

I would suggest implementing it as follows:

# BEGIN
if 2>/dev/null read -r h x < /etc/hostname; then
	source="/etc/hostname"
elif [ -z "${h:=$hostname}" ]; then
	source="/etc/conf.d/hostname"
else
	einfo "Using default system hostname"
	return 0
fi
ebegin "Setting hostname to $h from $source"
hostname "$h"
eend $? "Failed to set the hostname"
# END

Note that the (racy) file tests are not required because. In the case that the file is empty, read will return a non-zero status anyway. This approach would also print a meaningful source in the case that it defers to /etc/conf.d/hostname.
Comment 3 Kerin Millar 2022-06-08 07:49:28 UTC
I forgot to say that setting HOSTNAME (in upper case) in /etc/conf.d/net has not been documented for at least 10 years. I think that's long enough for admins to have contended with dispatch-conf.
Comment 4 Kerin Millar 2022-06-08 07:50:17 UTC
Apologies, /etc/conf.d/hostname, not /etc/conf.d/net. This is what happens when I comment before being caffeinated.
Comment 5 William Hubbs gentoo-dev 2022-06-08 18:55:35 UTC
I have cleaned up the hostname service script as follows.

https://github.com/OpenRC/openrc/commit/d2b31440

This removes the bashism and adds several more cleanups.

Thanks,

William
Comment 6 Kerin Millar 2022-06-08 19:06:56 UTC
Thanks, William.
Comment 7 Kerin Millar 2022-06-08 20:00:26 UTC
Pardon me for re-opening. I just want to point out that the commit has the redirection operator in the wrong place. It needs to be positioned so as to precede the read command, as was originally suggested. Otherwise, any errors spewed by a failing read won't necessarily be suppressed. I realise that this is counter-intuitive but it is true. That is all.
Comment 8 William Hubbs gentoo-dev 2022-06-09 14:50:06 UTC
I ran the following manual test before I made the change.

# echo $h

# read -r h _ 2> /dev/null < /etc/hostname
# echo $h
linux1
# unset h
# read -r h _ 2> /dev/null < /etc/host
# echo $?
2
# echo $h

#

The second read command above did not output any errors, so what am I
missing?

Thanks,

William
BUGZ ---------------------------------------------------
Comment 9 Kerin Millar 2022-06-09 16:43:59 UTC
Hi William. I could have sworn that I had a test case stashed somewhere that demonstrated it in one of the sh providers but it seems that I'm mistaken. Presently, all my tests show that the only thing that matters is that the stderr redirection is first (which isn't news). Perhaps my memory is starting to fail me. At any rate, sorry for the noise. Closing again!