On Wed, Dec 15, 2010 at 12:47 AM, OpenBSD Geek <[email protected]> wrote: > After correction, i'm agree it is a bit long, but it works. > Thank you very much for your posts! > > if [ $1 ] & [ $2 ]; then
Missing an ampersand, and if $1 or $2 evaluate to -t, some shells will return 1 if stdout is not a terminal. > > tempfile=`mktemp /tmp/tempfile.XXXX` > cp /etc/group $tempfile /tmp/`basename $0`.XXX is better it would suck if all scripts used tempfile as a prefix > > onlygroup=`mktemp /tmp/tempfile.XXXX` > cat $tempfile | grep ^$2 > $onlygroup cat file | grep == grep < file > > nogroup=`mktemp /tmp/tempfile.XXXX` > cat $tempfile | grep -v ^$2 > $nogroup > > cat $onlygroup | sed "s/$1//g" | \ > B B B B sed "s/ /,/g" | sed "s/,,/,/g" | sed "s/,$//g" >> $nogroup sed "s:,\?$1,\?::g" < $onlygroup > $nogroup This is just making it readable It doesnt address the fact that passwd/group should NOT be edited this way > > mv /etc/group /etc/group.old > cp $nogroup /etc/group No, dude. cp is not atomic > > chmod 644 /etc/group > chown root /etc/group > chgrp wheel /etc/group > > rm -f /tmp/tempfile.???? No. When you use mktemp, it returns the generated name: rm $tempfile $nogroup $onlygroup > > else > echo "Remove user from a group" > echo "Use : sh duig user group" > fi Please do not wrap your whole script in an if clause. Just return earlier. Even if you do correct these things, it's still horrible.
