Hi Roland, you wrote:
... CC:'ing Michelle Olson: Michelle... is there any preferred layout/style or something which I could as "template" for the language being used ? Thanks for taking this on, sounds like a very useful new document. I think the best template is chapter 7 of the Developer's Reference Guide. The DocBook4 XML sources are located here: http://dlc.sun.com/osol/docs/downloads/current/ In the devref-xml directory when tarball is expanded. Here is the ID for the chapter 7 <sect1 id="_7_2_Style_Guide" xreflabel="7.2 Style Guide"> HTML is here: http://www.opensolaris.org/os/community/on/devref_toc/devref_7/#7_2_style_guide Currently Mike Kupfer is doing some work on the Makefile and the guide, so be aware of that and sync with him if you have any trouble when you get to the processing stage. His changes are not yet in the doc consolidation, so just be aware that they're coming soon. The source for chapter 7 should get you started with the structures you're likely to need, based on my understanding of what you want to do. Let me know of any other ways I can help you out and thanks for taking initiative to document this information for the greater community. Regards, Michelle Olson OpenSolaris Documentation Leader Roland Mainz wrote: > Danek Duvall wrote: > >> On Thu, Jun 07, 2007 at 12:06:34AM +0200, Roland Mainz wrote: >> >>> Does Sun have any coding (style) guidelines/notes for Bourne/Korn/POSIX >>> shell scripts ? >>> >> No, only C (and, by some extension, C++) and Java. Shell, perl, and python >> are missing. >> >> >>> A quick review of some standard scripts like "bfu", "acr", some of the >>> SMF/Dtrace shell scripts and other stuff reveals some IMO not very "nice" >>> (where "nice" is more or less an understatement) constructs... >>> >> Oh, yes. Definitely. >> > > Ok... lets start (this is mainly about Korn shell/POSIX scripts but some > things apply to older shells like Bourne, too) ... > > ---- SNIP ---- SNIP ---- > * Use "print" and not "echo" for output. > "print" is guranteed to be a ksh builtin while "echo" may not be a > builtin (causing extra |fork()|+|exec()|), additionally "echo" may not > be portable (e.g. SysV vs. BSD "echo" differences) > > > * Avoid things like $ echo "foo" >xxx ; echo "bar" >>xxx ; echo "baz" > >>> xxx # >>> > Each of the redirections above trigger an > |open()|,|write()|,|close()|-sequence. It is much more efficient group > the rediction into a block, e.g. $ { echo "foo" ; echo "bar" ; echo > "baz" } >xxx # > > > * Use typed variables if possible. For example the following is very > inefficient since it transforms the integer values to strings and back > several times: > -- snip -- > a=0 > b=1 > c=2 > # more code > if [ $a -lt 5 -o $b -gt c ] ; then do_something ; fi > -- snip -- > This could be rewritten using ksh constructs: > -- snip -- > integer a=0 > integer b=1 > integer c=2 > if (( a < 5 || b > c )) ; then do_something ; fi > -- snip -- > > > * Use ksh style functions instead of Bourne-style functions if possible > (and local variables instead of spamming the global namespace) > > > * Use builtin math ksh instead of "expr" or "bc"/"dc" > > > * Use printf "%a" when passing floating-point values between scripts or > as outout of a function to avoid rounting errors when converting between > bases. > Example: > -- snip -- > function xxx > { > float val > > (( val=sin(5.) )) > printf "%a\n" val > } > float out > (( out=$(xxx) )) > xxx > print -- $out > -- snip -- > This will print: > -- snip -- > -0.9589242747 > -0x1.eaf81f5e09933226af13e5563bc6p-01 > -- snip -- > > > * _ALWAYS_ put variables into quotes when handling filenames, even if > the values are hardcoded or the values appear to be fixed. Otherwise at > least two things may go wrong: > - a malicious user may be able to exploit a script's inner working to > infect his/her own code > - a script may (fatally) misbehave for unexpected input (e.g. file names > with blanks and/or special symbols which are interpreted by the shell) > > > * Avoid the creation of temporary files and store the values in > variables intead. > Example: > -- snip -- > ls -1 >xxx > for i in $(cat xxx) ; do > do_something ; > done > -- snip -- > can be replaced with > -- snip -- > x="$(ls -1)" > for i in ${x} ; do > do_something ; > done > -- snip -- > Note that ksh93 supports binary variables which can hold any value. > > > * Use builtin commands if the shell provides them. For example ksh93s+ > (ksh93, version 's+') delivered with Solaris supports the following > builtins: > -- snip -- > basename > cat > chgrp > chmod > chown > cmp > comm > cp > cut > date > dirname > expr > fds > fmt > fold > getconf > head > id > join > ln > logname > mkdir > mkfifo > mv > paste > pathchk > rev > rm > rmdir > stty > tail > tee > tty > uname > uniq > wc > sync > -- snip -- > Those builtins can be enabled via $ builtin name_of_builtin # in shell > scripts (note that ksh93 builtins implement exact POSIX behaviour - some > commands in Solaris /usr/bin/ directory implement pre-POSIX behaviour. > Add /usr/xpg6/bin/:/usr/xpg4/bin before /usr/bin/ ${PATH} to test > whether your script works with the XPG6/POSIX versions) > > > * Use blocks and not subshells if possible, e.g. use $ { print "foo" ; > print "bar" ; } instead of $ (print "foo" ; print "bar") # - blocks are > faster since they do not require to save the subshell context (ksh93) or > trigger a shell child process (Bourne shell, bash, ksh88 etc.) > > > * Using -p" for starting non-interactive shell scipts is AFAIK a NO-OP, > e.g. > -- snip -- > #!/bin/ksh -p > -- snip -- > should be replaced with > -- snip -- > #!/bin/ksh > -- snip -- > > > * Store lists in arrays or associative arrays - this is usually easier > to manage. For example: > -- snip -- > x=" > /etc/foo > /etc/bar > /etc/baz > " > echo $x > -- snip -- > can be replaced with > -- snip -- > typeset -a mylist > mylist[0]="/etc/foo" > mylist[1]="/etc/bar" > mylist[2]="/etc/baz" > print ${x[*]} > -- snip -- > or (ksh93-style append entries to a normal (non-associative) array) > -- snip -- > typeset -a mylist > mylist+=( "/etc/foo" ) > mylist+=( "/etc/bar" ) > mylist+=( "/etc/baz" ) > print ${x[*]} > -- snip -- > > > * Use "[[ expr ]]" instead of "[ expr ]" if possible since it avoids > going through the whole pattern expansion/etc. machinery > > > * Use $( ... ) instead of `...` - `...` is an obsolete construct in ksh > scripts > > > * Make sure that yoour shell has a "true" builtin (like ksh93) when > executing endless loops like $ while true ; do do_something ; done # - > otherwise each loop cycle runs a |fork()|+|exec()|-cycle to run > /bin/true > > > * Always put the result of $( ... ) in quotes (e.g. "$( ... )" ) unless > there is a very good reason for not doing it > > > * Scripts should always set their PATH to make sure they do not use > alternative commands by accident > > > * Scripts should make sure that commands in optional packages are really > there, e.g. add a "precheck" block in scipts to avoid later failure when > doing the main job > > > * Check how boolean values are used in your application. For example: > -- snip -- > mybool=0 > # do something > if [ $mybool -eq 1 ] ; then do_something_1 ; fi > -- snip -- > could be rewritten like this: > -- snip -- > mybool=false # (valid values are "true" or "false", pointing > # to the builtin equivalents of /bin/true or /bin/false) > # do something > if ${mybool} ; then do_something_1 ; fi > -- snip -- > or > -- snip -- > integer mybool=0 # values are 0 or 1 > # do something > if (( mybool==1 )) ; then do_something_1 ; fi > -- snip -- > > > * Names of local variables which are not exported to the environment > should be lowercase while variable names which are exported to the > environment should be uppercase. > The only exception are global constants (=global readonly variables, > e.g. $ float -r M_PI=3.14159265358979323846 # (taken from <math.h>)) > which may be allowed to use uppercase names, too. > > > * Think about whether your application has to handle file names or > variables in multibyte locales and make sure all commands used in your > script can handle such charatcers (e.g. lots of commands in Solaris's > /usr/bin/ are _not_ able to handle such values - either use ksh93 > builtin constructs (which are guranteed to be multibyte-aware) or > commands from /usr/xpg4/bin and/or /usr/xpg6/bin) > > > * Use external filters like grep/sed/awk/etc. only if a significant > amount of data is processed by the filter. For example: > -- snip -- > if [ "$(echo "$x" | egrep '.*foo.*)" != "" ] ; then > do_something ; > done > -- snip -- > can be re-written using ksh93 builtin constructs, saving several > |fork()|+|exec()|'s: > -- snip -- > if [[ "${x}" == ~(E).*foo.* ]] ; then > do_something ; > done > -- snip -- > > > * Always use '{'+'}' when using variable names longer than one character > (to avoid problems with array or compund variables: > -- snip -- > print $foo > -- snip -- > should be rewritten to > -- snip -- > print ${foo} > -- snip -- > > > * Use "print -- ${varname}" when there is the slightest chance that the > variable "varname" may contain symbols like "-". Or better use "printf" > instead, for example > -- snip -- > integer fx > # do something > print $fx > -- snip -- > may fail if "f" contains a negative value. A better way may be to use > -- snip -- > inetger fx > # do something > print "%f" fx > -- snip -- > > > * Use $ export FOOBAR=val # instead of $ FOOBAR=val ; export FOOBAR # - > this is much faster > > > * Use compound variables or associative arrays to group similar > variables together. > For example: > -- snip -- > box_width=56 > box_height=10 > box_depth=19 > echo "${box_width} ${box_height} ${box_depth}" > -- snip -- > could be rewritten to ("associative array"-style) > -- snip -- > typeset -A -E box=( "width"=56 "height"=10 depth=19 ) > print -- "${box[width"]} ${box["height"]} ${box["depth"]}" > -- snip -- > or ("compound variable"-style > -- snip -- > box=( > float width=56 > float height=10 > float depth=19 > ) > print -- "${box.width} ${box.height} ${box.depth}" > -- snip -- > > > * Use a subshell (e.g. $ ( mycmd ) #) around places which use "set -- > $(mycmd)" and/oror "shift" unless the variable affected is either a > local one or if it's guranteed that this variable will no longer be used > (be carefull for loadable functions, e.g. ksh/ksh93's "autoload" !!!!) > > > * Cleanup after yourself. For example ksh/ksh93 have an EXIT trap which > is very usefull for this. > Note that the EXIT trap is executed for a subshell and each subshell > level can run it#s own EXIT trap, for example > $ (trap "print bam" EXIT ; (trap "print snap" EXIT ; print "foo")) # > will print > -- snip -- > foo > snap > bam > -- snip -- > > > * Put constant values into readonly variables, for example: > -- snip -- > float -r M_PI=3.14159265358979323846 > -- snip -- > or > -- snip -- > float M_PI=3.14159265358979323846 > readonly M_PI > -- snip -- > > > * If you create more than one temporary file create an unique subdir for > these files and make sure the dir is writeable. Make sure you cleanup > after yourself (unless you are debugging). > > > * Be carefull with using TABS in script code, they are not portable > between editors or platforms > > > * if youhave multiple points where your application exists with an error > message create a central function for this, e.g. > -- snip -- > if [ -z "$tmpdir" ] ; then > print -u2 "mktemp failed to produce output; aborting." > exit 1 > fi > if [ ! -d $tmpdir ] ; then > print -u2 "mktemp failed to create a directory; aborting." > exit 1 > fi > -- snip -- > should be replaced with > -- snip -- > function fatal_error > { > print -u2 "${progname}: $@" > exit 1 > } > # do something (and save ARGV[0] to variable "progname") > if [ -z "$tmpdir" ] ; then > fatal_error "mktemp failed to produce output; aborting." > fi > if [ ! -d "$tmpdir" ] ; then > fatal_error "mktemp failed to create a directory; aborting." > fi > -- snip -- > > > * Think about using $ set +o unset # by default (or at least during the > script's development phase) to catch errors where variables are used > when they are not set (yet), e.g. > -- snip -- > $ (set +o unset ; print ${foonotset}) > /bin/ksh93: foonotset: parameter not set > -- snip -- > ---- SNIP ---- SNIP ---- > > > Uhm... better I create a DocBook/XML document from that... this list > will become very long (above is only from "bfu", "acr" etc. but there > are many many more scripts in OS/Net which could be improved a lot...) > > ... CC:'ing Michelle Olson: Michelle... is there any preferred > layout/style or something which I could as "template" for the language > being used ? > > ---- > > Bye, > Roland > >
