See below :)

On 3 May 2012 16:32, COX Derek <derek....@alstom.com> wrote:

>  You may well find you generate a bunch of contradictory ideas about
> style, efficiency over simplicity etc
>
> Oh, and some people can get mighty pedantic too  J
>
>
>
> Constructive observations
>
>
>
> -          Why assign $# to argcount rather than just checking $# ?
>
Good point, This could have been much shorter, but I kind of like this way
of doing things as it documents itself, not everyone knows what $# means,
but "argcount = $#" says  it all. You are right though If concise code is
the key, then documentation is often not an issue. Point taken all the
same, I just do things this way.


>
>
> -          Error checking missing after commands (eg groupadd, useradd)
>
>
>

True, but thats why I added the syscheck output - as I didn't really know
how to do this bit


>  -          Consider shell substitution to get group and members rather
> than two new awk processes
>
> group=${line%%:*}
>
> members=${line##*:}
>

Wow  - can you explain these!! They look awesome!


>
>
> -          Reconsider `groups $member | grep –q $group`  - it will match
> superstrings of the group. Eg “groups | grep sales” would match group
> “salesman”.  grep –wq should do it. (or a user called lesales ……)
>
>
>

Hmm I hear you, the original code was something like this:

grep -c ^$group:

so that wasnt a problem before i hacked up the condition tests..



>  Good luck ;)
>
>
>

Thanks for your feedback, you've added at least 50% more awesomeness to my
coding skills (can you just explain those regexes for me in the shell
substitition?).

Always good to get some criticisms, makes me a better coder in the end :)


>
>
>
>
>
>
>
>
> *From:* peterboro-boun...@mailman.lug.org.uk [mailto:
> peterboro-boun...@mailman.lug.org.uk] *On Behalf Of *Richard Forth
> *Sent:* 03 May 2012 15:28
>
> *To:* Peterborough LUG - No commercial posts
> *Subject:* Re: [Peterboro] BASH CODING: Bulk User and Group Adder
>
>
>
> Aaargh - new version!!
>
>
> http://pastebin.com/yL2pvayv
>
> Corrected a schoolboy error
>
> for line in `cat bugalist`
>
> to
>
> for line in `cat $1`
>
>
>  On 3 May 2012 15:25, Richard Forth <richard.fo...@gmail.com> wrote:
>
> Thanks Jonathan,
>
> I've incorporated your suggestions in a new version:
>
> http://pastebin.com/v9k27RSA
>
> Cheers!
>
> Richard
>
>
>
> On 3 May 2012 15:07, Johnathon Tinsley <johnat...@positive-internet.com>
> wrote:
>
> Hi Richard,
>
> Really quick scan, but it looks like you're loading a variable to run a
> if on, basing it on grep (line 16 etc). You can do this directly, so
> something like:
>
> if grep -q ^$group: /etc/group; then
>  echo "found one or more!"
> else;
>  echo "didn't find any :("
> fi
>
> The -q means grep is quiet, only returning via exit codes. Relevant man
> page snippet:
> -q, --quiet, --silent
> Quiet;  do  not  write  anything  to  standard   output.    Exit
> immediately with zero status if any match is found, even if an error was
> detected.  Also see the -s  or  --no-messages  option. (-q is specified
> by POSIX.)
>
> HTH,
>
> Johnathon
>
>
> On 03/05/12 14:43, Richard Forth wrote:
> > Hi Peeps,
> >
> > I wanted to share with you a working BASH script that basically takes a
> > textfile that looks kinda like this:
> >
> > sales:mark,sally,dave,sue,steve
> > marketing:jason,philip,eric,steve
> > ...
> >
> > and processes each line - lets take the first line as example:
> >
> > Checks to see if sales exists already in /etc/group
> > If not creates it.
> > then checks to see if each user exists, if they do
> > it checks to see if they are already in the group,
> > if not it adds it to the group
> > if the user doesnt exist it creates the user and assigns a random 8
> > character password, then adds it to the group.
> > If you want one particular user to be in more than one group thats ok
> > just add it to whatever line for that group (see steve in the example).
> >
> > Its only 49 lines of code, including documentation notes.
> >
> > Probably totally impractical in real life but it was an exercise in bash
> > coding, with this aim in mind.
> >
> > If anyone wants a copy, I've created a public pastebin:
> > http://pastebin.com/uPxsZcGr
> >
> > the textfile was originally called bugalist
> >
> >
> >      8          for line in `cat bugalist`
> >
> >
> > You can probably modify line 8 to be $1 or set up some code to check for
> > arguments etc, which I now realise I missed out, and didnt document
> > either (damn).
> >
> > Anyway just in case you wonder what "bugalist" was:
> >
> > ~/Dev_Area/bash $ cat bugalist
> > u5:john,tessa,richard,steve
> > u6:dorris,erran,jason,steve
> > ~/Dev_Area/bash $
> >
> > Sample output:
> > ~/Dev_Area/bash $ sudo ./buga
> > ===========
> > Processing Line:
> > u5:john,tessa,richard,steve
> > ===========
> >         Creating group 'u5' ...
> >                 Members: john,tessa,richard,steve
> >                         Creating user 'john' ...
> >                         Password for user 'john' set to: vM6oElDG
> >                         Adding user 'john' to group 'u5' ...
> >                         uid=1002(john) gid=1003(john)
> > groups=1003(john),1002(u5)
> >
> >                         Creating user 'tessa' ...
> >                         Password for user 'tessa' set to: t9jt2uRR
> >                         Adding user 'tessa' to group 'u5' ...
> >                         uid=1003(tessa) gid=1004(tessa)
> > groups=1004(tessa),1002(u5)
> >
> >                         Creating user 'richard' ...
> >                         Password for user 'richard' set to: vZddQN4g
> >                         Adding user 'richard' to group 'u5' ...
> >                         uid=1004(richard) gid=1005(richard)
> > groups=1005(richard),1002(u5)
> >
> >                         Creating user 'steve' ...
> >                         Password for user 'steve' set to: RG8fdZjG
> >                         Adding user 'steve' to group 'u5' ...
> >                         uid=1005(steve) gid=1006(steve)
> > groups=1006(steve),1002(u5)
> >
> > ===========
> > Processing Line:
> > u6:dorris,erran,jason,steve
> > ===========
> >         Creating group 'u6' ...
> >                 Members: dorris,erran,jason,steve
> >                         Creating user 'dorris' ...
> >                         Password for user 'dorris' set to: qDTgCcxn
> >                         Adding user 'dorris' to group 'u6' ...
> >                         uid=1006(dorris) gid=1008(dorris)
> > groups=1008(dorris),1007(u6)
> >
> >                         Creating user 'erran' ...
> >                         Password for user 'erran' set to: JOL47SrN
> >                         Adding user 'erran' to group 'u6' ...
> >                         uid=1007(erran) gid=1009(erran)
> > groups=1009(erran),1007(u6)
> >
> >                         Creating user 'jason' ...
> >                         Password for user 'jason' set to: 3UG416G6
> >                         Adding user 'jason' to group 'u6' ...
> >                         uid=1008(jason) gid=1010(jason)
> > groups=1010(jason),1007(u6)
> >
> >                         User 'steve' already exists.
> >                         Adding user 'steve' to group 'u6' ...
> >                         uid=1005(steve) gid=1006(steve)
> > groups=1006(steve),1002(u5),1007(u6)
> >
> >
> > Enjoy!
> >
> > Feedback welcome.
> >
> > Enjoy!
> > Rich
> >
> >
> >
> >
> >
> >
>
> > _______________________________________________
> > Peterboro mailing list
> > Peterboro@mailman.lug.org.uk
> > https://mailman.lug.org.uk/mailman/listinfo/peterboro
>
>  --
> All postal correspondence to:
> The Positive Internet Company, 24 Ganton Street, London. W1F 7QY
>
> The Positive Internet Company Limited is registered in England and Wales.
> Registered company number: 3673639. VAT no: 726 7072 28.
> Registered office: Northside House, Mount Pleasant, Barnet, Herts, EN4 9EE.
>
>
> _______________________________________________
> Peterboro mailing list
> Peterboro@mailman.lug.org.uk
> https://mailman.lug.org.uk/mailman/listinfo/peterboro
>
>
>
>
>
> ------------------------------
> CONFIDENTIALITY : This e-mail and any attachments are confidential and may
> be privileged. If you are not a named recipient, please notify the sender
> immediately and do not disclose the contents to another person, use it for
> any purpose or store or copy the information in any medium.
>
> _______________________________________________
> Peterboro mailing list
> Peterboro@mailman.lug.org.uk
> https://mailman.lug.org.uk/mailman/listinfo/peterboro
>
_______________________________________________
Peterboro mailing list
Peterboro@mailman.lug.org.uk
https://mailman.lug.org.uk/mailman/listinfo/peterboro

Reply via email to