Shell scripting review

Posted on 2013-12-22
Last Modified: 2014-01-05
Hi ,

I have written a script based on collegues description, i just not sure whether it was well written .

I need your expert advice on the below code to check for any mistakes since i dont have the setup to test here in my environment.

#written by magento
#Code written based on assumptions below
# $1 = cointype - It is the directory created using (mkdir /home/coinname/.coinname)
# $2 = github url (
# $3 = node ip added in coinname.conf (echo "addnode=$3")
#validating the arguments 
if test $# != 3
    echo "Usage $0: coin_type github_location  coin_upstream_node_ip" 1>&2
    exit 1
#generating random password
random_passwd=`tr -dc A-Za-z0-9_ < /dev/urandom | head -c 16 | xargs`
#steps as per description
adduser poolname $random_passwd
echo "user added with password:$random_passwd"
cd /home/poolname
git clone $2
cd coinname/src
make -f makefile.unix USE_UPNP=-
chown coinname:coinname /home/coinname
mkdir /home/coinname/$1
#declaring conf filepath
#consolidating the conf 
function generate_coinconf
	echo "rpcuser=coinnamerpc" >> $conf
	echo "rpcpassword=43462346236090980423423256342" >> $conf
	echo "rcpallowip=*" >> $conf
	echo "server=1" >> $conf
	echo "listen=1" >> $conf
	echo "daemon=1" >> $conf
	echo "addnode=$3" >> $conf
 #writing the conf
#simply running coinname
sudo su coinname
./coinname 2>&1 > /home/coinname/$1/coinname.log
#getting the port
echo "port details below:"
./coinname --help | awk 'NF > 0 { print $1 }' test.txt | sed -ne '/-port/,/-rpcport/p'
#finally controversing alias cp
\cp -R /home/template/
echo "done"

Open in new window

Question by:magento
LVL 19

Accepted Solution

simon3270 earned 400 total points
ID: 39735566
Few specific points:
- when setting random_password, you don't need the xargs at the end.

- I don't think that having a function "generate_coinconf" adds much to the script - you could just as easily have the lines in the code.  You might do this to minimise clutter (just a simple "generate_coinconf" rather than lots of lines of text), but you would then move the function definition away (at the top of the script?).

- "sudo su coinname" would try, as root, to become the coinname user, but then be at a  shell prompt.  I assume that's not what you want. should the text on the next line (./coinname...') be on this line?

- in the first line that runs ./coinname, standard error will be sent to the console, and only standard out will be sent to the log file.  Is this what you want?  If you want both stdout and stderr to go to the log file, reverse the redirection statements, so you have
    ./coinname > /home/coinname/$1/coinname.log 2>&1

- In the second line that runs ./coinname, the awk line has a file to process (test.txt) so it won't process its standard input - the output from "./coinname --help" will be thrown away.

- Aliasses don't apply within shell scripts, sothere's no need for the \ in \cp.

- The "cp" doesn't have a target, just a source (/home/template).

Otherwise, my main comment would be that you have nowhere near enough error checking.  What happens if the adduser fails?  What if there is a problem running the "make"?  What if the coinname user doesn't already exist?
LVL 37

Assisted Solution

by:Gerwin Jansen
Gerwin Jansen earned 100 total points
ID: 39735788
You want to know whether your script is 'well written' - this is subjective, not objective. So you need to ask yourself what you consider well. As the above post is pointing out already, there are some syntax and logical issues in your script, they don't add to 'well' written scripts.

In general, I would:

- Start with pseudo code, this will save you time in the end and may prevent logic errors
- Think of how you want to test your script, write test scenarios if you can, including extreme cases (like 0 parameters, 3 parameters and 10 parameters for example)
- Write small piece of code, test
- Put pieces together, test

I usually keep my pseudo code as a comment.

Author Comment

ID: 39736312
Hi Simon,

Thanks for your advice , i will correct those redirections ,function and alias.

I assumed there wont be any problem with add user and those stuffs.

The awk need to work with the ./coinname --help only so i will change that too .


Featured Post

Microsoft Certification Exam 74-409

Veeam® is happy to provide the Microsoft community with a study guide prepared by MVP and MCT, Orin Thomas. This guide will take you through each of the exam objectives, helping you to prepare for and pass the examination.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Suggested Solutions

FreeBSD on EC2 FreeBSD ( is a robust Unix-like operating system that has been around for many years. FreeBSD is available on Amazon EC2 through Amazon Machine Images (AMIs) provided by FreeBSD developer and security office…
Join Greg Farro and Ethan Banks from Packet Pushers ( and Greg Ross from Paessler ( for a discussion about smart network …
In a previous video, we went over how to export a DynamoDB table into Amazon S3.  In this video, we show how to load the export from S3 into a DynamoDB table.
Get a first impression of how PRTG looks and learn how it works.   This video is a short introduction to PRTG, as an initial overview or as a quick start for new PRTG users.

895 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

11 Experts available now in Live!

Get 1:1 Help Now