Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I tried looking up some good example init scripts, but all that I found were excessively complicated. So I wrote my own simple init script targeting CentOS 6. I tried to follow LSB where I understood what it says. CentOS uses chkconfig instead of the LSB comment block.

This is my first serious shell script ever. What do you guys think?

#!/bin/sh
# chkconfig: 2345 80 20
# description: Perforce server

. /etc/profile.d/p4d.sh

command_line='p4d -d'

running() {
    pgrep -fx "$command_line" > /dev/null
}

start() {
    if ! running; then
        $command_line > /dev/null
    fi
}

stop() {
    if running; then
        p4 admin stop > /dev/null
    fi
}

case "$1" in
    start)
        start
    ;;
    stop)
        stop
    ;;
    restart|force-reload)
        stop
        start
    ;;
    status)
        if running; then
            echo "Running"
        else
            echo "Stopped"
            exit 3
        fi
    ;;
    *)
        echo "Usage: $0 {start|stop|restart|force-reload|status}"
        exit 1
    ;;
esac

exit 0
share|improve this question

1 Answer 1

It's clear, nicely formatted, and passes http://www.shellcheck.net/# with flying colors. I have only minor nitpicks.

The variable name command_line is too generic. If you make it more descriptive, for example start_command, then the purpose of the pgrep line becomes easier to understand: it should match whatever was used to start the command.

This maybe a matter of taste, but I like to place the ;; in case statements at the same indent level as the body of the statement, for example:

case "$1" in
    start)
        start
        ;;       # aligned with the commands, not the label
    stop)

I'm not really aware of a standard, but for what it's worth, Vim's auto-indent function (and auto-reindent with gg=G) indents this way. This writing style is also practical when folding is enabled in Vim, making long case statement blocks appear like this:

case "$1" in
    start)
        --- # folded lines
    stop)
        --- # folded lines
    restart|force-reload)
        --- # folded lines

Well, this is really a nitpick, and a matter of taste, your way is perfectly fine too.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.