• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 342
  • Last Modified:

Shell scripting review

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.

#!/usr/bin/sh
#written by magento
#Code written based on assumptions below
# $1 = cointype - It is the directory created using (mkdir /home/coinname/.coinname)
# $2 = github url (https://github.com/coinname/coinname)
# $3 = node ip added in coinname.conf (echo "addnode=$3")
#validating the arguments 
if test $# != 3
then
    echo "Usage $0: coin_type github_location  coin_upstream_node_ip" 1>&2
    exit 1
fi
#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
conf=/home/coinname/$1/coinname.conf
#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
 generate_coinconf
#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

0
magento
Asked:
magento
2 Solutions
 
simon3270Commented:
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?
0
 
Gerwin Jansen, EE MVETopic Advisor Commented:
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.
0
 
magentoAuthor Commented:
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 .

Thanks
0

Featured Post

Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Tackle projects and never again get stuck behind a technical roadblock.
Join Now