[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#139969: debian-policy: dubious shell programming advice re: set -e [patch included]



Package: debian-policy
Version: 3.5.6.1
Severity: normal

The sections which recommend the use of set -e are IMHO a bit off the
mark. The recommendations imply that failing silently is a Good Thing
and that not handling errors sensibly is okay in shell scripts.

In summary, relying on set -e to trap your errors for you results in
counter-intuitive behavior from the end user's perspective. If the
init.d script just runs and doesn't tell you the program you wanted to
start is not available, is that good behavior? If the script just dies
silently when your disk fills up, is that good behavior? No, these
errors should be trapped and at least result in a warning message.

(For example, I have filed a bug against e.g. the `bug' package for an
issue which is more or less caused by the present policy; see bug
#76127. I suspect there will be many more which just wait to be
discovered.)

As a proposed fix, I have reworded some parts of the policy, as
included in the patch below. This modifies the "should" and adds a
"may", with some related changes in earlier sections of the manual. I
also took the liberty to add comments to the /etc/init.d/bind example
where it breaks the new recommendation.

I am not familiar with the procedures for making a formal proposal for
an amendment to the policy. Somebody who is a Debian Developer could
perhaps take it from here if you agree with this.

Thank you for your time,

/* era */

Index: policy.sgml
===================================================================
RCS file: /cvs/debian-policy/debian-policy/policy.sgml,v
retrieving revision 1.67
diff -c -r1.67 policy.sgml
*** policy.sgml	2002/03/14 18:17:48	1.67
--- policy.sgml	2002/03/26 08:57:11
***************
*** 2486,2492 ****
  	  non-zero status if there is an error, so that the package
  	  management system can stop its processing.  For shell
  	  scripts this means that you <em>almost always</em> need to
! 	  use <tt>set -e</tt> (this is usually true when writing shell
  	  scripts, in fact).  It is also important, of course, that
  	  they don't exit with a non-zero status if everything went
  	  well.
--- 2486,2492 ----
  	  non-zero status if there is an error, so that the package
  	  management system can stop its processing.  For shell
  	  scripts this means that you <em>almost always</em> need to
! 	  use <tt>set -e</tt> (this is often true when writing shell
  	  scripts, in fact).  It is also important, of course, that
  	  they don't exit with a non-zero status if everything went
  	  well.
***************
*** 4720,4728 ****
--- 4720,4730 ----
  # Original version by Robert Leslie
  # &lt;rob@mars.org&gt;, edited by iwj and cs
  
+ # XXX: should perhaps print an error message
  test -x /usr/sbin/named || exit 0
  
  # Source defaults file.
+ # XXX: should perhaps try to trap errors in the defaults file
  PARAMS=''
  if [ -f /etc/default/bind ]; then
    . /etc/default/bind
***************
*** 5631,5637 ****
  	  should almost certainly start with <tt>set -e</tt> so that
  	  errors are detected.  Every script should use
  	  <tt>set -e</tt> or check the exit status of <em>every</em>
! 	  command.</p>
  
  	<p>
  	  The standard shell interpreter <tt>/bin/sh</tt> can be a
--- 5633,5644 ----
  	  should almost certainly start with <tt>set -e</tt> so that
  	  errors are detected.  Every script should use
  	  <tt>set -e</tt> or check the exit status of <em>every</em>
! 	  command.
! 	  For maximum usability,
! 	  it is recommended that
! 	  common error conditions
! 	  are caught and handled
! 	  even if <tt>set -e</tt> is used.</p>
  
  	<p>
  	  The standard shell interpreter <tt>/bin/sh</tt> can be a


-- System Information
Debian Release: 2.2
Kernel Version: Linux there 2.2.17 #1 Sun Jun 25 09:24:41 EST 2000 i586 unknown



-- 
To UNSUBSCRIBE, email to debian-policy-request@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org



Reply to: