Skip to main content
added 42 characters in body
Source Link
Gordon Davisson
  • 5.2k
  • 1
  • 21
  • 19

The problem was with tests like this:

if [ "$INPUT" -eq 1 ]; then

Since -eq tests for integer equality, it was failing if $INPUT was blank (or otherwise not a valid integer). In this case, we don't really need to test it as an integer (for example, we don't need to treat "01" "+1" and "1" as all being equal), it's safer to use a string comparison with =:

if [ "$INPUT" = 1 ]; then

(You could double-quote the 1 if you wanted, but it's not necessary.) BTW, I'd also recommend adding an else clause after all of the tests:

else
    echo "Unrecognized command: $INPUT"
    # Maybe exit here?
fi

(with appropriate translation, of course.) Another option is to use a case statement instead of the series of if/elif/etc:

case "$INPUT" in
    1) apache ;;
    2) mysql ;;
    ...
    0) exit ;;
    *) echo "Unrecognized command: $INPUT"
        # Maybe exit here?
        ;;
esac

It'd also be good to add some error-checking to the script. By default, if a command in a shell script fails, the rest of the script will just blindly continue on as though nothing were wrong. Take this section:

cd /etc/apache2/sites-available/

echo "<VirtualHost *:80>
    ...
    </VirtualHost>" > "$domain".conf
    

If the cd command fails for any reason, the echo will go ahead and create a .conf file in whatever directory you happen to be in at the time. Probably the simplest thing to do as add -e to the shebang line:

#!/bin/bash -e

which tells bash to exit if any command (other than those in a test or something like that) fails. Unfortunately, it's not very smart; it sometimes exits due to things that don't really seem like they should be errors at all, or fails to exit due to things that clearly are errors. For more infoa good explanation of why this is hard to get right, see BashFAQ #105: Why doesn't set -e (or set -o errexit, or trap ERR) do what I expected?

Individual-command error checks are really better, but a lot more work.

A few other semi-stylistic recommendations: use lower- or mixed-case variable names (input instead of INPUT) to avoid accidentally using one of the all-caps names with a special meaning; avoid echo -e since some versions will just print "-e" as part of the output (if you need special processing, use printf instead of echo); and indent your code (e.g. inside function definitions, loops, conditionals, etc) for readability.

The problem was with tests like this:

if [ "$INPUT" -eq 1 ]; then

Since -eq tests for integer equality, it was failing if $INPUT was blank (or otherwise not a valid integer). In this case, we don't really need to test it as an integer (for example, we don't need to treat "01" "+1" and "1" as all being equal), it's safer to use a string comparison with =:

if [ "$INPUT" = 1 ]; then

(You could double-quote the 1 if you wanted, but it's not necessary.) BTW, I'd also recommend adding an else clause after all of the tests:

else
    echo "Unrecognized command: $INPUT"
    # Maybe exit here?
fi

(with appropriate translation, of course.) Another option is to use a case statement instead of the series of if/elif/etc:

case "$INPUT" in
    1) apache ;;
    2) mysql ;;
    ...
    0) exit ;;
    *) echo "Unrecognized command: $INPUT"
        # Maybe exit here?
        ;;
esac

It'd also be good to add some error-checking to the script. By default, if a command in a shell script fails, the rest of the script will just blindly continue on as though nothing were wrong. Take this section:

cd /etc/apache2/sites-available/

echo "<VirtualHost *:80>
    ...
    </VirtualHost>" > "$domain".conf
    

If the cd command fails for any reason, the echo will go ahead and create a .conf file in whatever directory you happen to be in at the time. Probably the simplest thing to do as add -e to the shebang line:

#!/bin/bash -e

which tells bash to exit if any command (other than those in a test or something like that) fails. Unfortunately, it's not very smart; it sometimes exits due to things that don't really seem like they should be errors at all, or fails to exit due to things that clearly are errors. For more info, see BashFAQ #105: Why doesn't set -e (or set -o errexit, or trap ERR) do what I expected?

Individual-command error checks are really better, but a lot more work.

A few other semi-stylistic recommendations: use lower- or mixed-case variable names (input instead of INPUT) to avoid accidentally using one of the all-caps names with a special meaning; avoid echo -e since some versions will just print "-e" as part of the output (if you need special processing, use printf instead of echo); and indent your code (e.g. inside function definitions, loops, conditionals, etc) for readability.

The problem was with tests like this:

if [ "$INPUT" -eq 1 ]; then

Since -eq tests for integer equality, it was failing if $INPUT was blank (or otherwise not a valid integer). In this case, we don't really need to test it as an integer (for example, we don't need to treat "01" "+1" and "1" as all being equal), it's safer to use a string comparison with =:

if [ "$INPUT" = 1 ]; then

(You could double-quote the 1 if you wanted, but it's not necessary.) BTW, I'd also recommend adding an else clause after all of the tests:

else
    echo "Unrecognized command: $INPUT"
    # Maybe exit here?
fi

(with appropriate translation, of course.) Another option is to use a case statement instead of the series of if/elif/etc:

case "$INPUT" in
    1) apache ;;
    2) mysql ;;
    ...
    0) exit ;;
    *) echo "Unrecognized command: $INPUT"
        # Maybe exit here?
        ;;
esac

It'd also be good to add some error-checking to the script. By default, if a command in a shell script fails, the rest of the script will just blindly continue on as though nothing were wrong. Take this section:

cd /etc/apache2/sites-available/

echo "<VirtualHost *:80>
    ...
    </VirtualHost>" > "$domain".conf
    

If the cd command fails for any reason, the echo will go ahead and create a .conf file in whatever directory you happen to be in at the time. Probably the simplest thing to do as add -e to the shebang line:

#!/bin/bash -e

which tells bash to exit if any command (other than those in a test or something like that) fails. Unfortunately, it's not very smart; it sometimes exits due to things that don't really seem like they should be errors at all, or fails to exit due to things that clearly are errors. For a good explanation of why this is hard to get right, see BashFAQ #105: Why doesn't set -e (or set -o errexit, or trap ERR) do what I expected?

Individual-command error checks are really better, but a lot more work.

A few other semi-stylistic recommendations: use lower- or mixed-case variable names (input instead of INPUT) to avoid accidentally using one of the all-caps names with a special meaning; avoid echo -e since some versions will just print "-e" as part of the output (if you need special processing, use printf instead of echo); and indent your code (e.g. inside function definitions, loops, conditionals, etc) for readability.

Added note about error checking.
Source Link
Gordon Davisson
  • 5.2k
  • 1
  • 21
  • 19

The problem was with tests like this:

if [ "$INPUT" -eq 1 ]; then

Since -eq tests for integer equality, it was failing if $INPUT was blank (or otherwise not a valid integer). In this case, we don't really need to test it as an integer (for example, we don't need to treat "01" "+1" and "1" as all being equal), it's safer to use a string comparison with =:

if [ "$INPUT" = 1 ]; then

(You could double-quote the 1 if you wanted, but it's not necessary.) BTW, I'd also recommend adding an else clause after all of the tests:

else
    echo "Unrecognized command: $INPUT"
    # Maybe exit here?
fi

(with appropriate translation, of course.) Another option is to use a case statement instead of the series of if/elif/etc:

case "$INPUT" in
    1) apache ;;
    2) mysql ;;
    ...
    0) exit ;;
    *) echo "Unrecognized command: $INPUT"
        # Maybe exit here?
        ;;
esac

It'd also be good to add some error-checking to the script. By default, if a command in a shell script fails, the rest of the script will just blindly continue on as though nothing were wrong. Take this section:

cd /etc/apache2/sites-available/

echo "<VirtualHost *:80>
    ...
    </VirtualHost>" > "$domain".conf
    

If the cd command fails for any reason, the echo will go ahead and create a .conf file in whatever directory you happen to be in at the time. Probably the simplest thing to do as add -e to the shebang line:

#!/bin/bash -e

which tells bash to exit if any command (other than those in a test or something like that) fails. Unfortunately, it's not very smart; it sometimes exits due to things that don't really seem like they should be errors at all, or fails to exit due to things that clearly are errors. For more info, see BashFAQ #105: Why doesn't set -e (or set -o errexit, or trap ERR) do what I expected?

Individual-command error checks are really better, but a lot more work.

A few other semi-stylistic recommendations: use lower- or mixed-case variable names (input instead of INPUT) to avoid accidentally using one of the all-caps names with a special meaning; avoid echo -e since some versions will just print "-e" as part of the output (if you need special processing, use printf instead of echo); and indent your code (e.g. inside function definitions, loops, conditionals, etc) for readability.

The problem was with tests like this:

if [ "$INPUT" -eq 1 ]; then

Since -eq tests for integer equality, it was failing if $INPUT was blank (or otherwise not a valid integer). In this case, we don't really need to test it as an integer (for example, we don't need to treat "01" "+1" and "1" as all being equal), it's safer to use a string comparison with =:

if [ "$INPUT" = 1 ]; then

(You could double-quote the 1 if you wanted, but it's not necessary.) BTW, I'd also recommend adding an else clause after all of the tests:

else
    echo "Unrecognized command: $INPUT"
    # Maybe exit here?
fi

(with appropriate translation, of course.) Another option is to use a case statement instead of the series of if/elif/etc:

case "$INPUT" in
    1) apache ;;
    2) mysql ;;
    ...
    0) exit ;;
    *) echo "Unrecognized command: $INPUT"
        # Maybe exit here?
        ;;
esac

A few other semi-stylistic recommendations: use lower- or mixed-case variable names (input instead of INPUT) to avoid accidentally using one of the all-caps names with a special meaning; avoid echo -e since some versions will just print "-e" as part of the output (if you need special processing, use printf instead of echo); and indent your code (e.g. inside function definitions, loops, conditionals, etc) for readability.

The problem was with tests like this:

if [ "$INPUT" -eq 1 ]; then

Since -eq tests for integer equality, it was failing if $INPUT was blank (or otherwise not a valid integer). In this case, we don't really need to test it as an integer (for example, we don't need to treat "01" "+1" and "1" as all being equal), it's safer to use a string comparison with =:

if [ "$INPUT" = 1 ]; then

(You could double-quote the 1 if you wanted, but it's not necessary.) BTW, I'd also recommend adding an else clause after all of the tests:

else
    echo "Unrecognized command: $INPUT"
    # Maybe exit here?
fi

(with appropriate translation, of course.) Another option is to use a case statement instead of the series of if/elif/etc:

case "$INPUT" in
    1) apache ;;
    2) mysql ;;
    ...
    0) exit ;;
    *) echo "Unrecognized command: $INPUT"
        # Maybe exit here?
        ;;
esac

It'd also be good to add some error-checking to the script. By default, if a command in a shell script fails, the rest of the script will just blindly continue on as though nothing were wrong. Take this section:

cd /etc/apache2/sites-available/

echo "<VirtualHost *:80>
    ...
    </VirtualHost>" > "$domain".conf
    

If the cd command fails for any reason, the echo will go ahead and create a .conf file in whatever directory you happen to be in at the time. Probably the simplest thing to do as add -e to the shebang line:

#!/bin/bash -e

which tells bash to exit if any command (other than those in a test or something like that) fails. Unfortunately, it's not very smart; it sometimes exits due to things that don't really seem like they should be errors at all, or fails to exit due to things that clearly are errors. For more info, see BashFAQ #105: Why doesn't set -e (or set -o errexit, or trap ERR) do what I expected?

Individual-command error checks are really better, but a lot more work.

A few other semi-stylistic recommendations: use lower- or mixed-case variable names (input instead of INPUT) to avoid accidentally using one of the all-caps names with a special meaning; avoid echo -e since some versions will just print "-e" as part of the output (if you need special processing, use printf instead of echo); and indent your code (e.g. inside function definitions, loops, conditionals, etc) for readability.

Source Link
Gordon Davisson
  • 5.2k
  • 1
  • 21
  • 19

The problem was with tests like this:

if [ "$INPUT" -eq 1 ]; then

Since -eq tests for integer equality, it was failing if $INPUT was blank (or otherwise not a valid integer). In this case, we don't really need to test it as an integer (for example, we don't need to treat "01" "+1" and "1" as all being equal), it's safer to use a string comparison with =:

if [ "$INPUT" = 1 ]; then

(You could double-quote the 1 if you wanted, but it's not necessary.) BTW, I'd also recommend adding an else clause after all of the tests:

else
    echo "Unrecognized command: $INPUT"
    # Maybe exit here?
fi

(with appropriate translation, of course.) Another option is to use a case statement instead of the series of if/elif/etc:

case "$INPUT" in
    1) apache ;;
    2) mysql ;;
    ...
    0) exit ;;
    *) echo "Unrecognized command: $INPUT"
        # Maybe exit here?
        ;;
esac

A few other semi-stylistic recommendations: use lower- or mixed-case variable names (input instead of INPUT) to avoid accidentally using one of the all-caps names with a special meaning; avoid echo -e since some versions will just print "-e" as part of the output (if you need special processing, use printf instead of echo); and indent your code (e.g. inside function definitions, loops, conditionals, etc) for readability.