pidofproc function in functions script

Dan Nicholson dbn.lists at gmail.com
Wed Jul 18 05:54:49 PDT 2007


On 7/4/07, Dennis J Perkins <dennisjperkins at comcast.net> wrote:
> I played with pidofproc and this seems to work properly.  I don't know
> if it would integrate into your functions without any changes.

I've had a look, and I like what you've done. It may need to be
massaged a bit to maintain compatibility in the LFS bootscripts,
though. I also added lfs-dev in case some more eyes want to shine on
here.

> I used getopts instead of your while loop.  I discovered that t does not
> detect options after a parameter.

That's probably true. I'm a little uncomfortable using getopts since
we don't use it anywhere else, but it is in the POSIX spec, so it's
safe to use.

> I also included an exit code for a syntax error, but LSB doesn't call
> for it.  Since pidofproc is meant to be called by other functions, it
> might not be that useful anyway.

I like errors on syntax abuse.

> It also returns a list of found pids.  LSB specifies exit codes but is
> silent on return vaues.

I think that's the way to go.

> pidofproc()
> {
>     local base    # basename of program
>     local opt     # option currently being processed
>     local pid     # pid from pidfile
>     local pids    # list of pids to return to calling function
>     local pidfile # pidfile passed to pidofproc
>     local pidlist # list of pids to be returned to calling function

This is actually an important change. Currently the other functions
rely on the side effect that pidlist is global. We can change those
other functions (this way is much cleaner, IMO), but there may be
scripts out in the wild that depend on the current functionality. I
didn't find any in either of the current LFS or BLFS bootscripts,
though.

>     while getopts ":p:" opt
>     do
>         case $opt in
>             p ) pidfile=$OPTARG ;;
>             * ) log_error_msg $usage
>                 exit 2 ;;  # Syntax error
>         esac
>     done
>     shift $((OPTIND - 1 ))

You removed the -s option, which was kind of nice when you just wanted
to know if it was running and didn't care what the output was. I
suppose it's not LSB supported, but we may need it to be LFS supported
:) I haven't checked out the situation with the current scripts.

>     # If a pidfile was not passed, look for one in /var/run.
>     base=${process##*/}
>     ${pidfile:=/var/run/$base.pid} 2> /dev/null

I would sort of like to kill this section since I think it's kind of
evil to "guess" a pidfile when it wasn't specifically asked for. This
would change existing behavior for current bootscripts. statusproc
currently does something like this, too.

>     if [ -f "$pidfile" ]; then

I think this is not quite right. If I asked you to look at
/var/run/dbus.pid (as opposed to it just being guessed above), then
that is exactly what I want to be looking at. If it doesn't exist,
then it's not running from my perspective. The whole reason I passed
in the pidfile is so we don't just go blindly searching for any
process named dbus.

So, there needs to be two tests: if the variable is set (assuming the
previous munging is removed), then if it exists.

if [ -n "$pidfile" ]; then
    if [ -f "$pidfile" ]; then
...

>         # LSB specifies that only the first line of the pidfile should
> be read.
>         read pidlist < $pidfile

I see. We're not exactly LSB compliant, but that should be good enough.

>     else
>         # No pidfile.  Try to find pid anyway.
>         pidlist=$(pidof -s $$ -o $PPID -x $process)
>         if [ -z "$pidlist" ]; then
>             return 3  # Not running
>         fi
>     fi
> -
>     # Build list of pids of running processes.
>     for pid in "$pidlist"

pidlist can't be quoted here or it will result in a single argument
for all pids.

$ for pid in "1 2"; do echo x$pid; done
x1 2

>     do
>         if kill -0 "$pid" 2> /dev/null ; then
>             pids="$pids $pid"
>         fi
>     done
>
>     if [ -z "$pids" ]; then
>         return 1  # Not running, but pid found.
>     else
>         echo $pids
>     fi
> }

The rest all looks good to me.

--
Dan



More information about the lfs-support mailing list