NetBSD Problem Report #45130

From woods@once.weird.com  Sun Jul 10 05:04:14 2011
Return-Path: <woods@once.weird.com>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 8065F63B8E2
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 10 Jul 2011 05:04:14 +0000 (UTC)
Message-Id: <m1QfmBa-001EBfC@once.weird.com>
Date: Sat, 9 Jul 2011 22:04:06 -0700 (PDT)
From: "Greg A. Woods" <woods@planix.com>
Sender: "Greg A. Woods" <woods@once.weird.com>
To: gnats-bugs@gnats.NetBSD.org
Subject: /etc/locate.conf cannot deal with pathnames containing spaces
X-Send-Pr-Version: 3.95

>Number:         45130
>Category:       bin
>Synopsis:       /etc/locate.conf cannot deal with pathnames containing spaces
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    apb
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jul 10 05:05:00 +0000 2011
>Closed-Date:    Sun Jul 17 03:35:24 +0000 2011
>Last-Modified:  Sun Jul 17 03:35:24 +0000 2011
>Originator:     Greg A. Woods
>Release:        NetBSD-current
>Organization:
Planix, Inc.; Kelowna, BC; Canada
>Environment:
System: NetBSD
>Description:

	it seems locate.updatedb cannot be told to ignore files or
	directories which have whitespace in their pathnames, but such
	names are becoming increasingly common on shared file stores,
	and even on native systems

>How-To-Repeat:

	try to configure locate.updatedb to ignore path with spaces

#
#	testing locate.updatedb
#
ignore	/home/oldfiler/bogus stuff
ignore  /home/oldfiler/not a real name
ignorecontents	/Volumes/Macintosh HD Part 2/backups
ignorecontents	/Volumes/Macintosh HD Part 2/also ignore


>Fix:

	the only sane fix seems to me to be to only allow one parameter
	each time a configuration option is mentioned

	this may be controversial since it will potentially cause
	previous configurations to stop working

	however I don't see any good alternative since it is impossible
	to make any kind of quoting work with shell "for" statements at
	this level -- use of some kind of separator character is
	required (along with a change of the $IFS setting), but any of
	those too are of course valid characters in a pathname
	component.  (yes, ":" is a possible choice, but it too will
	cause problems for some folks!)

	the only other option is to rewrite in C and implement proper
	quoting (and escaping of quotes, etc.), but this is probably
	unnecessary since each of the important configuration options
	can appear multiple times in the locate.conf file, so
	restricting each one to specifying one pattern should suffice

Index: usr.bin/locate/locate/updatedb.sh
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/usr.bin/locate/locate/updatedb.sh,v
retrieving revision 1.11
diff -u -r1.11 updatedb.sh
--- usr.bin/locate/locate/updatedb.sh	23 Apr 2006 03:04:08 -0000	1.11
+++ usr.bin/locate/locate/updatedb.sh	10 Jul 2011 04:10:07 -0000
@@ -43,7 +43,8 @@
 					# for temp files
 TMPDIR=/tmp
 FCODES="/var/db/locate.database"	# the database
-CONF=/etc/locate.conf			# configuration file
+#CONF=${CONF:-/etc/locate.conf}		# for testing
+CONF=/etc/locate.conf			# the configuration file

 PATH="/bin:/usr/bin"

@@ -53,45 +54,46 @@

 # read configuration file
 if [ -f "$CONF" ]; then
-	exec 5<&0 < "$CONF"
-	while read com args; do
-		case "$com/$args" in /) continue;; esac	# skip blank lines
-		case "$com" in
-		'#'*)	;;			# lines start with # is comment
+	while read opt arg; do
+		case "$opt/$arg" in /) continue;; esac	# skip blank lines
+		case "$opt" in
+		'#'*)	
+			# a line starting with # is a comment
+			;;
 		searchpath)
-			SRCHPATHS="$SRCHPATHS $args";;
+			SRCHPATHS="$SRCHPATHS '$arg'"
+			;;
 		ignorefs)
-			for i in $args; do
-				case "$i" in
-				none)	ignorefs=;;
-				*)	fs=`echo "$i" | sed -e 's/^!/! -fstype /' -e t -e 's/^/-fstype /'`
-					ignorefs="${ignorefs:+${ignorefs} -o }${fs}"
-				esac
-			done;;
+			case "$arg" in
+			none)
+				ignorefs=
+				;;
+			*)
+				# fstypes cannot contain spaces!
+				fs=$(echo "$arg" | sed -e 's/^!/! -fstype /' -e t -e 's/^/-fstype /')
+				ignorefs="${ignorefs:+${ignorefs} -o }${fs}"
+				;;
+			esac
+			;;
 		ignore)
-			set -f
-			for i in $args; do
-				ignore="${ignore:+${ignore} -o }-path ${i}"
-			done
-			set +f;;
+			ignore="${ignore:+${ignore} -o }-path '${arg}'"
+			;;
 		ignorecontents)
-			set -f
-			for i in $args; do
-				ignore="${ignore:+${ignore} -o }-path ${i} -print"
-			done
-			set +f;;
+			ignore="${ignore:+${ignore} -o }-path '${arg}' -print"
+			;;
 		workdir)
-			if [ -d "$args" ]; then
-				TMPDIR="$args"
+			if [ -d "$arg" ]; then
+				TMPDIR="$arg"
 			else
-				echo "$CONF: workdir: $args nonexistent" >&2
-			fi;;
+				echo "$CONF: workdir: $arg nonexistent" >&2
+			fi
+			;;
 		*)
-			echo "$CONF: $com: unknown config command"	>&2
-			exit 1;;
+			echo "$CONF: $opt: unknown config option"	>&2
+			exit 1
+			;;
 		esac
-	done
-	exec <&5 5>&-
+	done < "$CONF"
 fi

 : ${SRCHPATHS:=/}			# directories to be put in the database
@@ -114,11 +116,11 @@

 # Make a file list and compute common bigrams.
 # Entries of each directory shall be sorted (find -s).
+# (the "extra" cat process lets us detect write errors)

 set -f
 (find -s ${SRCHPATHS} $lp $ignorefs $ignore $rp -print; true) | cat  >> "$FILELIST"
-if [ $? != 0 ]
-then
+if [ $? != 0 ]; then
 	exit 1
 fi

Index: usr.bin/locate/locate/Makefile
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/usr.bin/locate/locate/Makefile,v
retrieving revision 1.10
diff -u -r1.10 Makefile
--- usr.bin/locate/locate/Makefile	5 Oct 2005 06:29:03 -0000	1.10
+++ usr.bin/locate/locate/Makefile	10 Jul 2011 03:41:19 -0000
@@ -4,9 +4,10 @@
 PROG=	locate
 MAN=	locate.1 locate.updatedb.8

-FILES=updatedb.sh
-FILESNAME=locate.updatedb
-FILESDIR=/usr/libexec
-FILESMODE=${BINMODE}
+# XXX locate.conf.5 SHOULD be here as well!
+
+SCRIPTS=updatedb.sh
+SCRIPTSNAME=locate.updatedb
+SCRIPTSDIR=/usr/libexec

 .include <bsd.prog.mk>
Index: share/man/man5/locate.conf.5
===================================================================
RCS file: /cvs/master/m-NetBSD/main/src/share/man/man5/locate.conf.5,v
retrieving revision 1.7
diff -u -r1.7 locate.conf.5
--- share/man/man5/locate.conf.5	30 Apr 2008 13:10:57 -0000	1.7
+++ share/man/man5/locate.conf.5	10 Jul 2011 03:46:26 -0000
@@ -56,23 +56,25 @@
 .Pp
 The configuration options are as follows:
 .Bl -tag -width XXXXXX
-.It Sy ignore Ar pattern ...
-Ignore files or directories.
+.It Sy ignore Ar pattern
+Ignore a file or directory.
 When building the database,
-do not descend into files or directories
-which match one of the specified patterns.
-The matched files or directories are not stored to the database.
+do not descend into a file or directory
+which matches the specified pattern.
+The matched file or directory is not stored to the database.
+This option may appear multiple times.
 .Pp
 Default: Not specified.
-.It Sy ignorecontents Ar pattern ...
-Ignore contents of directories.
+.It Sy ignorecontents Ar pattern
+Ignore contents of a directory.
 When building the database,
-do not descend into files or directories
-which match one of the specified patterns.
-The matched files or directories themselves are stored to the database.
+do not descend into a file or directory
+which matches the specified pattern.
+The matched file or directory is itself stored to the database.
+This option may appear multiple times.
 .Pp
 Default: Not specified.
-.It Sy ignorefs Ar type ...
+.It Sy ignorefs Ar type
 Ignore file system by type,
 adding
 .Ar type
@@ -94,6 +96,7 @@
 the
 .Sy ignorefs
 list is cleared and all file systems are traversed.
+This option may appear multiple times.
 .Pp
 .Ar type
 is used as an argument to
@@ -108,8 +111,9 @@
 .Ed
 .Pp
 Default: !local cd9660 fdesc kernfs procfs
-.It Sy searchpath Ar directory ...
-Specify base directories to be put in the database.
+.It Sy searchpath Ar directory
+Specify a base directory name to be put in the database.
+This option may appear multiple times.
 .Pp
 Default: /
 .It Sy workdir Ar directory

>Release-Note:

>Audit-Trail:
From: David Laight <david@l8s.co.uk>
To: gnats-bugs@NetBSD.org
Cc: gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: bin/45130: /etc/locate.conf cannot deal with pathnames containing spaces
Date: Sun, 10 Jul 2011 08:59:12 +0100

 On Sun, Jul 10, 2011 at 05:05:00AM +0000, Greg A. Woods wrote:
 > >Number:         45130
 > >Category:       bin
 > >Synopsis:       /etc/locate.conf cannot deal with pathnames containing spaces
 ...
 > 	it seems locate.updatedb cannot be told to ignore files or
 > 	directories which have whitespace in their pathnames, but such
 > 	names are becoming increasingly common on shared file stores,
 > 	and even on native systems

 If might be possible to do a simpler fix by removing 'space' from IFS.

 	David

 -- 
 David Laight: david@l8s.co.uk

From: Alan Barrett <apb@cequrux.com>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/45130: /etc/locate.conf cannot deal with pathnames
 containing spaces
Date: Sun, 10 Jul 2011 10:29:49 +0200

 >	it seems locate.updatedb cannot be told to ignore files or
 >	directories which have whitespace in their pathnames, but such
 >	names are becoming increasingly common on shared file stores,
 >	and even on native systems

 Thanks for the patch.  However, it has another problem: Files or
 directories that have quotation marks in their names cannot be
 specified.

 I think that we should allow multiple arguments, as it says in
 the man page, but allow shell-style quoting within the arguments,
 like this:

 #
 #       testing locate.updatedb
 #
 ignore  /home/oldfiler/bogus\ stuff
 ignore  /home/oldfiler/"not a real name"
 ignorecontents  "/Volumes/Macintosh HD Part 2/backups"
 ignorecontents  '/Volumes/Macintosh HD Part 2/also ignore'

Responsible-Changed-From-To: bin-bug-people->apb
Responsible-Changed-By: apb@NetBSD.org
Responsible-Changed-When: Sun, 10 Jul 2011 08:31:09 +0000
Responsible-Changed-Why:
I'll fix it


From: "Alan Barrett" <apb@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/45130 CVS commit: src
Date: Sun, 10 Jul 2011 13:42:50 +0000

 Module Name:	src
 Committed By:	apb
 Date:		Sun Jul 10 13:42:50 UTC 2011

 Modified Files:
 	src/share/man/man5: locate.conf.5
 	src/usr.bin/locate/locate: updatedb.sh

 Log Message:
 Allow quoting of embedded shell metacharacters in locate.conf(5).

 The shell_quote function here is identical to that in postinstall
 and etcupdate.

 This should fix PR 45130 from Greg Woods.


 To generate a diff of this commit:
 cvs rdiff -u -r1.8 -r1.9 src/share/man/man5/locate.conf.5
 cvs rdiff -u -r1.11 -r1.12 src/usr.bin/locate/locate/updatedb.sh

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: apb@NetBSD.org
State-Changed-When: Sun, 10 Jul 2011 13:46:22 +0000
State-Changed-Why:
Does revision 1.12 of src/usr.bin/locate/locate/updatedb.sh fix this?


From: "Greg A. Woods" <woods@planix.com>
To: gnats-bugs@NetBSD.org
Cc: netbsd-bugs@netbsd.org,
	gnats-admin@netbsd.org
Subject: Re: bin/45130 (/etc/locate.conf cannot deal with pathnames containing spaces)
Date: Mon, 11 Jul 2011 18:16:49 -0700

 --pgp-sign-Multipart_Mon_Jul_11_18:16:29_2011-1
 Content-Type: text/plain; charset=US-ASCII
 Content-Transfer-Encoding: quoted-printable

 At Sun, 10 Jul 2011 13:46:24 +0000 (UTC), apb@NetBSD.org wrote:
 Subject: Re: bin/45130 (/etc/locate.conf cannot deal with pathnames contain=
 ing spaces)
 >=20
 > Synopsis: /etc/locate.conf cannot deal with pathnames containing spaces
 >=20
 > State-Changed-From-To: open->feedback
 > State-Changed-By: apb@NetBSD.org
 > State-Changed-When: Sun, 10 Jul 2011 13:46:22 +0000
 > State-Changed-Why:
 > Does revision 1.12 of src/usr.bin/locate/locate/updatedb.sh fix this?

 I think so -- I haven't tested it yet, but conceptually it looks right,
 and I assume you've tested it some!

 Thanks for reminding me about using 'read -r', 'set', and "$@" -- I
 forgot about how the latter could work with a "for" statement to keep
 quoting in place!

 I see you caught some other minor bugs too!  :-)

 The appearance of shell_quote() reminds me of this:

     http://mail-index.netbsd.org/tech-userlevel/2008/09/05/msg001147.html

 or at least adding "%q" or similar to printf(1) using shquote(3).  :-)
 (and maybe "%v" with several option flag using strvis(3))


 Now if we could just get every other *BSD and derivative to adopt this
 version of locate.conf(5) verbatim.  (OS X is of course where I first
 ran into the problem, and they're still using the hokey shell variables
 mechanism from FreeBSD for their /etc/locate.rc.)

 Next, on to maybe adopting some of FreeBSD's fixes for managing the size
 during processing of large locate lists.  I have 110 million files on my
 desktop machine, apparently, and that's on a less than half-full 1TB
 drive.  The find will probably run for more than 72 hours (at nice -10).


 --=20
 						Greg A. Woods

 +1 250 762-7675                                RoboHack <woods@robohack.ca>
 Planix, Inc. <woods@planix.com>      Secrets of the Weird <woods@weird.com>

 --pgp-sign-Multipart_Mon_Jul_11_18:16:29_2011-1
 Content-Type: application/pgp-signature
 Content-Transfer-Encoding: 7bit

 -----BEGIN PGP SIGNATURE-----
 Version: GnuPG v1.4.9 (NetBSD)

 iD8DBQBOG6BtZn1xt3i/9H8RApKeAJ9PN80laqJ5hBvt+xjtx5qxaUbn7ACgxOzw
 vLmN4IxIVuvFW8gkJM43cm0=
 =COfl
 -----END PGP SIGNATURE-----

 --pgp-sign-Multipart_Mon_Jul_11_18:16:29_2011-1--

From: "Greg A. Woods" <woods@planix.ca>
To: NetBSD GNATS <gnats-bugs@NetBSD.org>, Alan Barrett <apb@NetBSD.org>
Cc: 
Subject: Re: bin/45130 (/etc/locate.conf cannot deal with pathnames containing spaces)
Date: Tue, 12 Jul 2011 15:17:14 -0700

 --Multipart_Tue_Jul_12_15:17:10_2011-1
 Content-Type: text/plain; charset=US-ASCII

 Ooops!

 "Nothing works until it has been tested."  :-)

 Testing revealed you got the '!' stripping backwards for the "ignorefs"
 directive.

 Also, I added back the warning about running as root, just as a
 suggestion.  Indeed it should be a separate PR, but.... :-)

 Also, I separated the case clause ";;" terminators -- I really don't
 like scrunching code together too much -- makes it too hard to read.

 That's how I first found the cause of this new bug -- by first opening
 up the code a bit, then I very quickly spotted the "inversion".

 I removed a bit of extra, unnecessary, quoting (it makes syntax
 highlighting work better and cleans up the readability).  :-)

 I.e. these two must always produce the same value (and they do on NetBSD
 for both sh and ksh, and even for bash and zsh):

      BAR=$(echo "foo  bar  ")
      BAR="$(echo "foo  bar  ")"

 Same for assignments containing only variable expansions.

 I added a comment about the "extra" cat -- things like this are not
 always obvious, even to experts, and perhaps especially not to someone
 accustomed to getting rid of "extra" processes like this in novice shell
 code.  :-)  (Here I was surprised enough by it that I checked the CVS
 annotation and then the CVS log to be sure it was on purpose, then I
 went "Duh, of course!".)

 In some ways the quoted-parameter handling by use of the special case
 quoted "$@" should probably also be documented -- in my experience it's
 a relatively rare idiom (all too rare, perhaps).

 Finally I fixed a style nit with the "if" after the find call.

 PS, I didn't PGP-sign this one, thus I didn't have to use any encoding,
 hopefully to make this easier to pick out of Gnats, but given I CC'ed
 you directly, then perhaps I could have encoded and signed it.  I really
 wish Gnats did MIME properly.  :-)  I'd prefer to sign everything I send
 these days.

 -- 
 						Greg A. Woods
 						Planix, Inc.

 <woods@planix.com>       +1 250 762-7675        http://www.planix.com/


 --Multipart_Tue_Jul_12_15:17:10_2011-1
 Content-Type: text/plain; charset=US-ASCII

 Index: updatedb.sh
 ===================================================================
 RCS file: /cvs/master/m-NetBSD/main/src/usr.bin/locate/locate/updatedb.sh,v
 retrieving revision 1.12
 diff -u -u -r1.12 updatedb.sh
 --- updatedb.sh	10 Jul 2011 13:42:49 -0000	1.12
 +++ updatedb.sh	12 Jul 2011 22:15:21 -0000
 @@ -43,7 +43,7 @@
  					# for temp files
  TMPDIR=/tmp
  FCODES="/var/db/locate.database"	# the database
 -CONF=/etc/locate.conf			# configuration file
 +CONF=${CONF:-/etc/locate.conf}		# the configuration file

  PATH="/bin:/usr/bin"

 @@ -62,54 +62,76 @@
  #
  shell_quote()
  {
 -	local result=''
 +	local result=
  	local arg
  	for arg in "$@" ; do
  		# Append a space if necessary
 -		result="${result}${result:+ }"
 +		#
 +		result=${result}${result:+ }
 +
  		# Convert each embedded ' to '\'',
  		# then insert ' at the beginning of the first line,
  		# and append ' at the end of the last line.
 -		result="${result}$(printf "%s\n" "$arg" | \
 -			sed -e "s/'/'\\\\''/g" -e "1s/^/'/" -e "\$s/\$/'/")"
 +		#
 +		result=${result}$(printf "%s\n" "$arg" | \
 +			sed -e "s/'/'\\\\''/g" -e "1s/^/'/" -e "\$s/\$/'/")
  	done
  	printf "%s\n" "$result"
  }

 +if [ "$(id -u)" -eq 0 ]; then
 +	echo ">>> WARNING"
 +	echo ">>> Executing updatedb as root.  This WILL reveal all filenames"
 +	echo ">>> on your machine to all login users, which may be a security risk."
 +fi 1>&2
 +
  # read configuration file
  if [ -f "$CONF" ]; then
  	while read -r com args; do
  		case "$com" in
  		''|'#'*)
 -			continue ;;	# skip blank lines and comment lines
 +			continue	# skip blank lines and comment lines
 +			;;
  		esac
  		eval "set -- $args"
  		case "$com" in
  		searchpath)
 -			SRCHPATHS="${SRCHPATHS}${SRCHPATHS:+ }$(shell_quote "$@")";;
 +			SRCHPATHS=${SRCHPATHS}${SRCHPATHS:+ }$(shell_quote "$@")
 +			;;
  		ignorefs)
  			for i in "$@"; do
  				fs=
  				case "$i" in
 -				none)	ignorefs=;;
 -				\!*)	fs="! -fstype $(shell_quote "$i")";;
 -				*)	fs="-fstype $(shell_quote "${i#?}")";;
 +				none)
 +					ignorefs=
 +					;;
 +				\!*)
 +					fs="! -fstype $(shell_quote "${i#!}")"
 +					;;
 +				*)
 +					fs="-fstype $(shell_quote "${i}")"
 +					;;
  				esac
  				case "$fs" in
 -				'')	;;
 -				*)	ignorefs="${ignorefs:+${ignorefs} -o }${fs}";;
 +				'')
 +					;;
 +				*)
 +					ignorefs="${ignorefs:+${ignorefs} -o }${fs}"
 +					;;
  				esac
  			done;;
  		ignore)
  			for i in "$@"; do
 -				q="$(shell_quote "$i")"
 +				q=$(shell_quote "$i")
  				ignore="${ignore:+${ignore} -o }-path ${q}"
 -			done;;
 +			done
 +			;;
  		ignorecontents)
  			for i in "$@"; do
 -				q="$(shell_quote "$i")"
 +				q=$(shell_quote "$i")
  				ignore="${ignore:+${ignore} -o }-path ${q} -print"
 -			done;;
 +			done
 +			;;
  		workdir)
  			if [ $# -ne 1 ]; then
  				echo "$CONF: workdir takes exactly one argument" >&2
 @@ -117,10 +139,12 @@
  				TMPDIR="$1"
  			else
  				echo "$CONF: workdir: $1 nonexistent" >&2
 -			fi;;
 +			fi
 +			;;
  		*)
  			echo "$CONF: $com: unknown config command" >&2
 -			exit 1;;
 +			exit 1
 +			;;
  		esac
  	done < "$CONF"
  fi
 @@ -145,11 +169,12 @@

  # Make a file list and compute common bigrams.
  # Entries of each directory shall be sorted (find -s).
 +# (note: the "extra" cat process lets us detect write errors)

  (eval "find -s ${SRCHPATHS} $lp $ignorefs $ignore $rp -print"; true) \
  	| cat >> "$FILELIST"
 -if [ $? != 0 ]
 -then
 +
 +if [ $? != 0 ]; then
  	exit 1
  fi


 --Multipart_Tue_Jul_12_15:17:10_2011-1--

From: "Alan Barrett" <apb@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/45130 CVS commit: src/usr.bin/locate/locate
Date: Wed, 13 Jul 2011 07:58:35 +0000

 Module Name:	src
 Committed By:	apb
 Date:		Wed Jul 13 07:58:35 UTC 2011

 Modified Files:
 	src/usr.bin/locate/locate: updatedb.sh

 Log Message:
 Comments and white space changes, inspired by Greg Woods' remarks
 in PR 45130, but not directly copied from the patch in the PR.


 To generate a diff of this commit:
 cvs rdiff -u -r1.13 -r1.14 src/usr.bin/locate/locate/updatedb.sh

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Alan Barrett <apb@netbsd.org>
To: NetBSD GNATS <gnats-bugs@netbsd.org>
Cc: 
Subject: Re: bin/45130 (/etc/locate.conf cannot deal with pathnames
 containing spaces)
Date: Wed, 13 Jul 2011 10:07:22 +0200

 On Tue, 12 Jul 2011, Greg A. Woods wrote:
 > Testing revealed you got the '!' stripping backwards for the 
 > "ignorefs" directive.

 Oops!  I fixed that in a different way, by introducing a new variable.

 > Also, I added back the warning about running as root, just as a 
 > suggestion.  Indeed it should be a separate PR, but.... :-)

 The original code didn't have such a warning, and I didn't notice 
 it in your patch.  If we want something like that, then I'd 
 suggest testing for user != "nobody", not uid == 0.  This sort of 
 change needs discussion in tech-userlevel; please suggest it there 
 if you want to pursue it.

 > Also, I separated the case clause ";;" terminators -- I really 
 > don't like scrunching code together too much -- makes it too 
 > hard to read.

 I have done some of what you suggested.

 > I removed a bit of extra, unnecessary, quoting (it makes syntax 
 > highlighting work better and cleans up the readability).  :-)
 >
 > I.e. these two must always produce the same value (and they do 
 > on NetBSD for both sh and ksh, and even for bash and zsh):
 >
 >     BAR=$(echo "foo  bar  ")
 >     BAR="$(echo "foo  bar  ")"

 I find it more readable when the extra quotes are present.  It's a 
 clear signal to the reader that the value is not subjected to word 
 splitting.

 >I added a comment about the "extra" cat

 OK.

 >In some ways the quoted-parameter handling by use of the special case
 >quoted "$@" should probably also be documented -- in my experience it's
 >a relatively rare idiom (all too rare, perhaps).

 "$@" is much too common to deserve commenting, but I commented
 the use of eval "set -- $args".

 >Finally I fixed a style nit with the "if" after the find call.

 OK.

 --apb (Alan Barrett)

State-Changed-From-To: feedback->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 17 Jul 2011 03:35:24 +0000
State-Changed-Why:
Fixes applied - if there are more needed, please write in and I'll reopen.


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.