>>>>> "Roland" == Roland Mainz <roland.mainz at nrubsig.org> writes:
Roland> I'd like to get some feedback (some kind of pre-review) on the
Roland> new shell code style guide
I think there's some good stuff in there. There are a few guidelines I
disagree with; I'll discuss those below, where I comment on specific
items.
I have a couple general concerns. One is about the organization of the
guide. The other is about distinguishing requirements from
recommendations.
The first thing I would focus on in organization. The current
organization is pretty random. This makes it hard to hold in one's
head. It also makes it more likely that redundant or conflicting items
will be added over time.
Organizing the items by topic area will make it easier to write up, in a
general way, a prose explanation of what the issues are for that area.
You can then get to specifics and examples.
At the very least, I would have sections on
- hardening (variable quoting, filenames starting with "-", etc)
- i18n
- performance
As far as requirements versus recommendations, the usual convention is
that "must" means a requirement, and "should" indicates a
recommendation.
I'm particularly concerned about the performance-related items. Showing
ways that developers can improve performance is great. Requiring that
developers adhere to certain stylistic conventions for code that is not
performance-critical will just encourage cynicism about the guide.
Roland> Is there any community ratification process for such documents ?
There's no formal one for ON. If we go by historical precedent, then
that sort of decision would be delegated to the C-team.
Here are my comments on the individual items up through "Always put the
result of $(...) in quotes". I've got comments on many of the items
after that, but it'll probably take me several days to find time to
write them up.
* Intro
The text says that the guide is for "new SMF shell code". Assuming
that the current target is wider than that, this text needs updating.
I'm skeptical about pointing people at the POSIX shell standard. I
haven't looked at the shell standard, but if it's anything like other
standards documents that I've read, it's written for conciseness, not
ease of comprehension. Most ON developers are probably unfamiliar
with the finer points of the standard, and for most ON scripts, they
don't need to be.
* Basic Format:
Scripts should follow the same encoding rules as C source, which I
believe means ASCII only, no UTF-8. I would like to see us move to
UTF-8 encoding, but I think we first need to make sure that such a
change would not introduce problems (e.g., due to old tools that don't
understand UTF-8).
* Interpreter magic
I believe that the Solaris ksh has been modified so that -p is no
longer needed. I suppose it's okay to mention -p anyway, but I would
not list it as a requirement.
* "Harden the script..."
This deserves to be its own section, with discussion of issues such as
- spaces in file names
- wildcard characters
- quoting
- filenames starting with "-"
- backticks versus $(...)
(I guess the backticks point is not really about hardening, but I'd
put it here as part of the general discussion on quoting.)
* "Use builtin commands..."
I object pretty strongly to this item. Script writers should not have
to track what commands are built in for which versions of which shell.
That sort of knowledge adds too much complexity and too much
brittleness to the scripts. At most, there can be a recommendation
for the script writer to set up PATH in a certain way. Other than
that, it should be up to the shell itself to figure out when to use a
builtin and when to use fork/exec.
There may be exceptions, where it makes sense to put in special effort
to tune a script, and to force some operation to use a builtin instead
of fork/exec. But those sorts of optimizations should be done only
after doing the analysis to understand where the time in the script is
going.
* "use long options for "set""
Editorial suggestion to improve readability: minimize the use of
inline examples, especially for command invocations. I think your
quoting conventions (e.g., the "#" at the end) are likely to confuse
people. Put each example command on its own line, e.g.,
Instead of
$ set -x
use
$ set -o xtrace
Similarly, your example scripts in the item "Use ksh-style function"
should not be jammed into a couple lines.
* "Always put the result of $(...) in quotes"
This is too strong. For example, if the $(...) expansion is used in
for f in $(...); do
then you probably *don't* want the results in quotes.
Rather than express this as a blanket directive, have a section on
quoting rules (as suggested above), mention the issues about blanks
(or metacharacters) in file names, and give a couple examples where
this can cause problems. If the quoting guidelines are the same for
$(...) and variable expansion, you'll only need to give the quoting
guidelines once.
mike
