"Mark J. Nelson" wrote: > > "Mark J. Nelson" wrote: > >>>> Did you find any issues? > >>> Yes... the ksh scripts should be "ksh93 -n <scriptname>" clean (which > >>> does some simple lint-like checking) - which isn't AFAIK done yet... > >>> ... which leads to a question: Is there a webrev where I can look at the > >>> changes ? > >> > >> Caveat: this webrev is PRELIMINARY. It is a chance for the SCM project > >> team members to be our own first reviewers. The real webrev (which you'll > >> see on tools-discuss) will be generated and published next week, after we > >> process the team feedback from this one. > >> > >> The current, preliminary webrev is at > >> http://cr.opensolaris.org/~richlowe/scm-review_90/ > > > > Do you still accept quick cleanup patches for "webrev"&co. ? > > Depending on nature and complexity, "yes," with the caveat that, when we > generate the external (to the project team) webrev next week, we'll freeze > what we take until we integrate.
Ok... attached is an updated webrev which kills most of the "ksh93 -n <script" warnings, the issue described in http://www.opensolaris.org/os/project/shell/shellstyle/#use_ksh_style_function_syntax and some other risky stuff (e.g. lack of quoting). If the patch is Ok I'd like to get a step further, e.g. ... - The script currently uses stuff like $ print "hello $world" but doesn't check whether the variable "world" may contain backslashes which are interpreted. The simple fix is to use $ printf "hello %s\n" "$world" # or $ print -f "hello %s\n" "$world" # - Do we really need "eval" in all cases ? I've commented at least in ~~two cases that using "nameref" is a more secure since any special characters or unexpected input won't blow-up the whole script. - The entity encoding can be replaced with ksh93's printf "%H" "<string>" - Half of the awk usage and AFAIK all usage of "sed" can be replaced with ksh93 builtin functionality (which will make the script more "portable" (or better: Doesn't depend on external utilties)). For example $ print "hello world" | sed 's/world/fish/' # can be replaced with $ (x="hello world" ; x="${x/world/fish}" ; print "$x") # etc. ... or $ $ (x="hello world" ; x="${x/~(E)(.*)(w.*d)/\2}" ; print "$x") # fetches the 2nd word matching the extended regular expression "w.*d" (e.g. the \2 points to the content of the 2nd (pattern)) - Some variables look like file lists... IMO it may be interesting to use array variables instead. - Usage of grep/egrep/fgrep can be replaced by builtin pattern matching, e.g. [[ "str" == ~(E)foo.*bar ]] matches the string "str" against the extended regualr expression pattern "foo.*bar" > After we integrate, I anticipate opening onnv-scm back up to ongoing work, > until such time as external folks are better able to integrate to the > "real" gate. But don't chisel that into stone. Ok... ---- Bye, Roland -- __ . . __ (o.\ \/ /.o) roland.mainz at nrubsig.org \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL +49 641 7950090 (;O/ \/ \O;) -------------- next part -------------- A non-text attachment was scrubbed... Name: webrev.sh Type: application/x-sh Size: 68632 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/scm-migration-dev/attachments/20080531/28501c00/attachment.sh>
