The init script for mpd comes with a get_config() function: get_config() { x=$1 test -e ${CFGFILE} || return 1 sed -n \ -e '/^[ \t]*'${x}'/{s:^[ \t]*'${x}'[ \t]\+... \ ${CFGFILE} } That's needed, essentially, because mpd doesn't let you override the PID file location on the command-line. However, it does let you run the daemon in the foreground via a command-line parameter. If you do that, OpenRC can background the process and manage the PID file for you, eliminating the need to dig through the config file. The resulting init script is a little simpler; get_config can be deleted, and you wind up with this chunk of variables: extra_started_commands='reload' command=/usr/bin/mpd command_args="--no-daemon ${CFGFILE}" command_background=true required_files=${CFGFILE} pidfile=/run/${RC_SVCNAME}.pid description="Music Player Daemon" You can then eliminate a portion of your custom mpd-0.18.conf.patch -- the part that messes with the pid_file setting. Finally, for the sake of portability, it's nice to use start-stop-daemon in the reload function: reload() { ebegin "Reloading ${RC_SVCNAME}" start-stop-daemon --pidfile $pidfile --signal HUP eend $? } (otherwise you require sys-process/procps at runtime for "kill")
Michael, I think you can commit this change if it works for you - maybe together with a 0.20.11 version bump?
(In reply to Andreas Sturmlechner from comment #1) > Michael, I think you can commit this change if it works for you - maybe > together with a 0.20.11 version bump? I don't use mpd myself, so I'm not confident saying that it works for me. I've been auditing init scripts: a lot of get_config() functions are exploitable -- and even though this one isn't -- I figured I'd offer some drive-by suggestions that I noticed in the process.
(In reply to Michael Orlitzky from comment #0) It would be great to have your improvements committed. I believe it will also solve bug #645664.
(In reply to Michael Orlitzky from comment #0) > The init script for mpd comes with a get_config() function: > > get_config() { > x=$1 > test -e ${CFGFILE} || return 1 > sed -n \ > -e '/^[ \t]*'${x}'/{s:^[ \t]*'${x}'[ \t]\+... \ > ${CFGFILE} > } > > That's needed, essentially, because mpd doesn't let you override the PID > file location on the command-line. However, it does let you run the daemon > in the foreground via a command-line parameter. If you do that, OpenRC can > background the process and manage the PID file for you, eliminating the need > to dig through the config file. The resulting init script is a little > simpler; get_config can be deleted, and you wind up with this chunk of > variables: > > extra_started_commands='reload' > command=/usr/bin/mpd > command_args="--no-daemon ${CFGFILE}" > command_background=true > required_files=${CFGFILE} > pidfile=/run/${RC_SVCNAME}.pid > description="Music Player Daemon" > > You can then eliminate a portion of your custom mpd-0.18.conf.patch -- the > part that messes with the pid_file setting. > > Finally, for the sake of portability, it's nice to use start-stop-daemon in > the reload function: > > reload() { > ebegin "Reloading ${RC_SVCNAME}" > start-stop-daemon --pidfile $pidfile --signal HUP > eend $? > } > > (otherwise you require sys-process/procps at runtime for "kill") feel free to submit a pull request though :)
I actually ammended the reload() function as I agree that s-s-d is better than the direct kill, as for pidfile path, I think this is better to leave everything as is.
