6
\$\begingroup\$

I started self-learning Bash scripting yesterday, and I finished my first successful "advance" script.

Goal

To create an interactive step-by-step script to that allowed customizing various parts of the process of creating permanent swapfiles. I based my idea off of Ask Ubuntu Answer's #1 and #2.

Background

While creating it, I extended ideas such as allowing exiting from the script at any time and formatting the output.

You will see that I use different styles throughout the code (i.e. case vs if-elif-else, etc...). That deals with me learning as I coded, and having issues trying to get the syntax right while learning. I want to consolidate my different styles - probably into cases.

Additional Concerns I have

I know that all of you will suggest any issues that you see, and I will be happy about that; however, I am curious about a few things.

  1. Is this code portable? [Edit] Mostly anyone with a Linux Distro should be able to use this script.
  2. I put a lot of effort into formatting the output and making it look easy to read. I tried to separate the output of my code vs the output of other commands (i.e. adding >> and>>>>). Is it good enough? Would you suggest anything else? etc?

Possible Extensions

The code works as intended; however, I do plan on extending it in the future. These could affect possible responses - some response might make these harder or easier to implement, or they might have no effect at all. Anyways, if I extend the code later on to accommodate more use-cases, these are what I believe I could do in the future:

  1. Allow a non-interactive script mode that uses arguments for customizations
  2. Allow single/batch removal of swapfiles.
  3. Allow easier testing of each pathway of the interactive script
  4. Allow color and bold support.

Code

GitHub Repo at ciscorucinski/Swapfile-Utils

For unmodified code, check out the Protected Branch at 1_200606_StackExchange-CodeReview

Note: Only the variable declaration of help_text on the GitHub link is different due to a mistake on my side. No escaped tabs and newlines in GitHub

For updated code, check out the Development Branch at develop

#!/bin/bash
help_text="/n>> Documentation:\n\n\t
Usage: $(basename "0ドル") [-h]\n\t 
Creates a new permanent swapfile\n\n\t
where:\n\t\t
 -h displays help text"
ask_for_size_allocation="
>>>> Amount to allocate for new swapfile? (default: 4G) : "
ask_for_swapfile_name="
>>>> Filename for new swapfile? (default: '/swapfile') : "
line="# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #"
shopt -s extglob
case 1ドル in
 ?(-)@(-h|-help) )
 echo -e help_text
 ;;
esac
retry=true
while $retry; do
 echo -e -n ">>>> Are you sure you want to create a new swapfile? (Y / N):"
 read yes_no
 case $yes_no in
 [Yy]|[Yy][Ee][Ss] )
 echo -e "\n$line"
 echo -e "Current Swapfiles:\n"
 sudo swapon -s 
 echo -e "$line"
 retry=false
 ;;
 [Nn]|[Nn][Oo]|[Qq][Uu][Ii][Tt] )
 echo -e ">> Exiting..."
 exit 0
 ;;
 * )
 echo -e ">> Error: invalid response\n"
 retry=true
 esac
done
echo -e ""
echo -e ">> Step 1: Size Allocation"
echo -e -n $ask_for_size_allocation
read swap_size
if [ -z "${swap_size}" ]; then
 swap_size="4G"
elif [[ $swap_size =~ [1-9][0-9]*[mMgG] ]]; then
 :
elif [[ $swap_size =~ [Qq][Uu][Ii][Tt] ]]; then
 echo -e ">> Exiting..."
 exit 0
else
 echo -e ">> Invalid Size: ${swap_size^^}. Exiting..."
 exit 1
fi
echo -e ""
echo -e ">> Step 2: File Name"
echo -e -n $ask_for_swapfile_name
read swap_name
if [ -z "${swap_name}" ]; then
 swap_name="/swapfile"
elif [[ $swap_size =~ [Qq][Uu][Ii][Tt] ]]; then
 echo -e ">> Exiting..."
 exit 0
elif [[ $swap_name =~ [/]+([0-9a-zA-Z]|[_-]) ]]; then
 :
elif [[ $swap_name =~ [+([0-9a-zA-Z]|[_-])] ]]; then 
 swap_name="/$swap_name" 
else
 echo -e ">> Invalid Pattern: $swap_name. Exiting..."
 exit 1
fi
echo -e ""
echo -e -n ">>>> Continue? '$swap_name' (${swap_size^^}) will be created. (Y / N):"
read yes_no
case $yes_no in
 [Yy]|[Yy][Ee][Ss] )
 echo -e""
 echo -e ">> 1. Creating swapfile..."
 echo -e ""
 echo -e "$line"
 sudo fallocate -l $swap_size $swap_name
 sudo chmod 600 $swap_name
 sudo mkswap $swap_name
 echo -e "$line"
 echo -e ""
 echo -e ">> 2. Enabling swapfile..."
 sudo swapon $swap_name
 echo -e ">> 3. Swapfile added."
 echo -e ""
 echo -e "$line"
 echo -e "Current Swapfiles:"
 echo -e ""
 sudo swapon -s 
 echo -e "$line"
 echo -e ""
 ;;
 [Qq][Uu][Ii][Tt]|[Nn]|[Nn][Oo]|[*])
 echo -e ">> Exiting..."
 exit 0
 ;;
esac
echo -e -n ">>>> Make swapfile permanent? (Y / N):"
read yes_no
case $yes_no in
 [Yy]|[Yy][Ee][Ss] )
 echo -e "$swap_name none swap sw 0 0" | sudo tee -a /etc/fstab > /dev/null
 echo -e ""
 echo -e ">> 4. Created permanent swapfile. Modified '/etc/fstab'"
 echo -e -n ">>>> Do you want to view '/etc/fstab?' (Y / N):"
 read yes_no
 case $yes_no in
 [Yy]|[Yy][Ee][Ss] )
 lenght=${#swap_name}
 echo -e ""
 echo -e "$line"
 cat /etc/fstab 
 echo -e "$line"
 ;;
 *)
 echo -e ">> Exiting..."
 exit 0
 ;;
 esac
 ;;
 [Qq][Uu][Ii][Tt] )
 echo -e ">> Exiting..."
 exit 0 
 ;;
 *)
 echo -e ">> 4. Created temp swapfile."
 echo -e ">> Exiting..."
 exit 0
 ;;
esac
shopt -u extglob

Output

$ bash new_swapfile.sh
>>>> Are you sure you want to create a new swapfile? (Y / N):y
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
Current Swapfiles:
[sudo] password for ciscorucinski: 
Filename Type Size Used Priority
/swapfile_ext3 file 262140 238384 -5
/swapfile_ext7 file 131068 106376 -3
/swapfile file 8388604 476148 -6
/swapfile_ext6 file 131068 68840 -4
/swapfile_ext5 file 131068 115304 -2
/dev/dm-1 partition 1003516 0 -7
/swapfile_ext8 file 131068 0 -8
/swapfile_ext9 file 131068 0 -9
/swapfile_ext10 file 131068 0 -10
/swapfile_ext11 file 131068 0 -11
/swapfile_ext12 file 131068 0 -12
/swapfile_ext13 file 131068 0 -13
/swapfile_ext file 524284 0 -14
/swapfile_ext14 file 131068 0 -15
/swapfile_ext15 file 131068 0 -16
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
>> Step 1: Size Allocation
>>>> Amount to allocate for new swapfile? (default: 4G) :128m
>> Step 2: File Name
>>>> Filename for new swapfile? (default: '/swapfile') :/swapfile_ext16
>>>> Continue? '/swapfile_ext16' (128M) will be created. (Y / N):y
>> 1. Creating swapfile...
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
Setting up swapspace version 1, size = 128 MiB (134213632 bytes)
no label, UUID=94e452f1-b252-4ea2-8d5f-e23634e34081
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
>> 2. Enabling swapfile...
>> 3. Swapfile added.
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
Current Swapfiles:
Filename Type Size Used Priority
/swapfile_ext3 file 262140 238384 -5
/swapfile_ext7 file 131068 106376 -3
/swapfile file 8388604 476148 -6
/swapfile_ext6 file 131068 68840 -4
/swapfile_ext5 file 131068 115304 -2
/dev/dm-1 partition 1003516 0 -7
/swapfile_ext8 file 131068 0 -8
/swapfile_ext9 file 131068 0 -9
/swapfile_ext10 file 131068 0 -10
/swapfile_ext11 file 131068 0 -11
/swapfile_ext12 file 131068 0 -12
/swapfile_ext13 file 131068 0 -13
/swapfile_ext file 524284 0 -14
/swapfile_ext14 file 131068 0 -15
/swapfile_ext15 file 131068 0 -16
/swapfile_ext16 file 131068 0 -17
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
>>>> Make swapfile permanent? (Y / N):y
>> 4. Created permanent swapfile. Modified '/etc/fstab'
>>>> Do you want to view '/etc/fstab?' (Y / N):y
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
# /etc/fstab: static file system information.
#
# Use 'blkid' to print the universally unique identifier for a
# device; this may be used with UUID= as a more robust way to name devices
# that works even if disks are added and removed. See fstab(5).
#
# <file system> <mount point> <type> <options> <dump> <pass>
/dev/mapper/ubuntu--vg-root / ext4 errors=remount-ro 0 1
# /boot/efi was on /dev/nvme0n1p1 during installation
UUID=D6C3-F1E1 /boot/efi vfat umask=0077 0 1
/dev/mapper/ubuntu--vg-swap_1 none swap sw 0 0
/swapfile none swap sw 0 0
/swapfile_ext3 none swap sw 0 0
/swapfile_ext5 none swap sw 0 0
/swapfile_ext6 none swap sw 0 0
/swapfile_ext7 none swap sw 0 0
/swapfile_ext8 none swap sw 0 0
/swapfile_ext9 none swap sw 0 0
/swapfile_ext10 none swap sw 0 0
/swapfile_ext11 none swap sw 0 0
/swapfile_ext13 none swap sw 0 0
/swapfile_ext14 none swap sw 0 0
/swapfile_ext15 none swap sw 0 0
/swapfile_ext16 none swap sw 0 0
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #
asked Jul 30, 2018 at 17:14
\$\endgroup\$

2 Answers 2

5
\$\begingroup\$

fallocate and swap files

You should not use fallocate for creating swap files. It's not supported.

From the mkswap manpage:

Note that a swap file must not contain any holes. Using cp(1) to
create the file is not acceptable. Neither is use of fallocate(1) on
file systems that support preallocated files, such as XFS or ext4, or
on copy-on-write filesystems like btrfs. It is recommended to use
dd(1) and /dev/zero in these cases. Please read notes from swapon(8)
before adding a swap file to copy-on-write filesystems.

And from the swapon manpage:

You should not use swapon on a file with holes. This can be seen in
the system log as
 swapon: swapfile has holes.
The swap file implementation in the kernel expects to be able to write
to the file directly, without the assistance of the filesystem. This
is a problem on preallocated files (e.g. fallocate(1)) on filesystems
like XFS or ext4, and on copy-on-write filesystems like btrfs.

It follows that, while fallocate may be faster than dd, it's not suitable for creating swap files and not supported by swap-related tools.


# # # # # ... ?

You should limit this to the width of the display (or default to 80, which is pretty common as a default width for terminals). There's no reason to have it extend beyond the visible width (and possibly wrap around). You can obtain the width from the COLUMNS special variable, and get a substring:

bash-4.4$ echo "$COLUMNS"
66
bash-4.4$ line="${line:0:$COLUMNS}"
bash-4.4$ echo "$line"
# # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # # #

Prompts for read

A echo -n "<prompt text>" before read is unnecessary, as read has the -p option:

 -p PROMPT output the string PROMPT without a trailing newline before
 attempting to read

Case-insensitive case

Use the nocasematch option:

If the nocasematch shell option (see the description of shopt in The Shopt Builtin) is enabled, the match is performed without regard to the case of alphabetic characters.

So:

shopt -s nocasematch
case $yes_no in
 y|yes) ...
 ;;
 n|no|quit) ...

And so on.

This option also applies to the ==/!=/=~ tests in [[ ... ]].


The shopt -u extglob is unnecessary.


Exit after --help

It's fairly standard to have commands exit without doing anything if asked to print help instead of continuing on to act on other options. And if you plan to extend it for non-interactive usage, then you'll likely add support for options as well. So you may want to print help for all unsupported options, and for options which were incorrectly given. So a new function:

usage() {
 echo "$help_text"
 exit "${1:-0}" # default exit status 0 if `1ドル` isn't given
}

An example option would look like:

case 1ドル in
 -n)
 [[ -z 2ドル ]] && usage 1 # error: need a second arg with `-n`, so exit 1
 do_something with "2ドル"
 ;;
 -h|--help)
 usage # asked for help, exit 0
 ;;
 -*) # error: unrecognized option, so exit 1
 usage 1
 ;;
esac

(Of course, if you do add more options, you should look into getopts for parsing them.)


  1. Allow a non-interactive script mode that uses arguments for customizations
  2. Allow single/batch removal of swapfiles.
  3. Allow easier testing of each pathway of the interactive script

In that case, I'd strongly suggest that you extract out the actual actions (listing swap files, creating swap files, mounting them, etc.) into individual functions. This will allow you to mix and match actions based on options, allowing for easier testing and batch processing.

So the code might look like:

list_swapfiles() {
 printf "\n%s\n" "$line"
 printf "%s\n\n" "Current Swapfiles:"
 sudo swapon -s
 printf "\n%s\n" "$line"
}
# after parsing options
case 1ドル in)
 -l|--list)
 list_swapfiles
 exit
 ;;
 ...
esac
# no options, assume interactive
while true; do
 read -p ">>>> Are you sure you want to create a new swapfile? (Y / N):" yes_no
 case $yes_no in
 y|yes) 
 list_swapfiles
 break
 ;;
 ...
answered Aug 1, 2018 at 4:32
\$\endgroup\$
8
  • 1
    \$\begingroup\$ In addition to using the correct exit status, I'd also redirect usage to the standard error stream unless the help was requested: usage 1 >&2. \$\endgroup\$ Commented Aug 1, 2018 at 11:54
  • \$\begingroup\$ @muru What is the scope of the shopt commands? If I use a shopt command then will it go back to the original usage after my script ends? Or does the life of that shopt command continue after my script ends? \$\endgroup\$ Commented Aug 1, 2018 at 12:11
  • 1
    \$\begingroup\$ @ChristopherRucinski the scope of the shopt built-in is the shell in which it was ran, which is the shell that was started to execute the script. See also superuser.com/a/176788/334516 \$\endgroup\$ Commented Aug 1, 2018 at 17:19
  • \$\begingroup\$ @muru I was looking at implementing your suggestions for the read command, and I say that people suggest to always use read -r. Do you also suggest that? My code does not disallow back-slashes for filenames. \$\endgroup\$ Commented Aug 1, 2018 at 17:34
  • \$\begingroup\$ @ChristopherRucinski I didn't notice you were reading filenames using read. In that case, please use IFS= read -r. \$\endgroup\$ Commented Aug 1, 2018 at 17:55
7
\$\begingroup\$

Portability

You mentioned portability as one of your top concerns. The commands used by the script limit it to certain systems:

  • swapon, mkswap: these are common in Linux systems
  • fallocate: available in Linux, supporting certain filesystems depending on the version
  • sudo: available in a wide range of systems but not always installed by default

This script is not portable, and I think that's not a real concern. It's designed to run in Linux systems with the supported filesystems. I think it wouldn't make sense to try to make it "portable".


In less specialized scripts that conceptually could make sense in many systems, using echo -e would be a portability concern. echo without flags works reliably, but its flags are not equally well supported in all systems. It's best to write scripts in a way to use only echo without any flags.

Broken help message

This is clearly a bug:

echo -e help_text

You probably intended this instead:

echo -e "$help_text"

And even then, the output would be pretty weird:

/n>> Documentation:
Usage: script.sh [-h]
Creates a new permanent swapfile
where:
 -h displays help text

Probably this is not what you intended.

Readability

I find the script very hard to read. For example this:

help_text="/n>> Documentation:\n\n\t
Usage: $(basename "0ドル") [-h]\n\t 
Creates a new permanent swapfile\n\n\t
where:\n\t\t
 -h displays help text"

Probably you intended something more like this:

help_text="
>> Documentation:
 Usage: $(basename "0ドル") [-h]
 Creates a new permanent swapfile
 where:
 -h displays help text"

Which will output:

>> Documentation:
 Usage: a.sh [-h]
 Creates a new permanent swapfile
 where:
 -h displays help text

There is no need for escape sequences like \n and \t, you can write them directly inside strings, a lot easier to read, and now you can echo "$help_text" without the -e flag and it just works.

Keep it simple

It's a good rule of thumb to keep things as simple as possible. For example you used the -e in all echo statements, even though most of them don't need it. I suggest to question the purpose of every single element, and cut out anything that isn't essential.


What is going on here?

?(-)@(-h|-help) )

Why not simply this:

-h|-help )

Some usability features can be simpler too. For example:

[Yy]|[Yy][Ee][Ss] )

How about like this instead:

[Yy]|yes|Yes )

The second is a lot easier to read. Sure, it's not the same thing, because the first version will match yEs too and the second won't. But is that really a problem?

Quoting

One of the most important things to learn properly in Bash scripting is how quoting works. It's not that complicated though. Whenever you use a variable as command arguments, they should be within double-quotes.

For example these statements are unsafe:

case 1ドル in
while $retry; do

They should be:

case "1ドル" in
while "$retry"; do

sudo in scripts

It's not great when a script needs to use sudo, because it can be annoying to get prompted for a password unexpectedly.

This script cannot do anything useful without sudo. It would be better to remove all the sudo from inside the script, and make it required to call the script itself with sudo. Add a simple check at the top of the script:

if [ "$(id -u)" != 0 ]; then
 echo "This script must be run as root or with sudo 0ドル" >&2
 exit 1
fi

Check yourself

There is a nice online service where you can paste your shell script and it points out bugs and safety issues, called shellcheck.net.

answered Jul 30, 2018 at 20:37
\$\endgroup\$
10
  • \$\begingroup\$ Also, there is no exit 0 with the help case. The idea behind ?(-)@(-h|-help) is that will allow a user to type -h, --h, -help, or --help to get help. I have seen this double-hyphen usage for help quite a few times and wanted to emulate it somewhat. Do you think I should not do that? \$\endgroup\$ Commented Jul 31, 2018 at 8:35
  • 1
    \$\begingroup\$ @ChristopherRucinski wow. I didn't know it means that. It's not common to support both -help and --help. I suggest to stick to the GNU convention of -h and --help. That will be simpler. And simple is good. \$\endgroup\$ Commented Jul 31, 2018 at 8:57
  • \$\begingroup\$ I agree. I recently found that the convention for double-dash is to use a word or phrase (--help) and the single-dash is for a letter (-h). With that it is uncommon to see --h and -help. I didn't know that, so I will use your suggestion \$\endgroup\$ Commented Jul 31, 2018 at 15:09
  • \$\begingroup\$ can you please provide resources I can review for your comment Whenever you use a variable as command arguments, they should be within double-quotes. I don't understand when I should and shouldn't double-quote variables, and I don't fully understand why they are unsafe, yet. \$\endgroup\$ Commented Jul 31, 2018 at 15:28
  • 1
    \$\begingroup\$ @muru that came from the manpage on my system, indeed that was too specific to mention without the source. Thanks, updated. \$\endgroup\$ Commented Aug 1, 2018 at 5:35

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.