Bruce Korb wrote: > Hi Jim, > > On 01/03/11 02:56, Jim Meyering wrote: >> @@ -85,14 +85,14 @@ v= >> # then try "git describe", then default. >> if test -f $tarball_version_file >> then >> - v=`cat $tarball_version_file` || exit 1 >> + v=`cat $tarball_version_file` || v= >> case $v in >> *$nl*) v= ;; # reject multi-line output >> [0-9]*) ;; >> *) v= ;; >> esac >> test -z "$v" \ >> - && echo "$0: WARNING: $tarball_version_file seems to be damaged" 1>&2 >> + && echo "$0: WARNING: $tarball_version_file is missing or damaged" 1>&2 >> fi > > Please notice that before "cat" runs, the file is tested for existence. > When "cat" does run, any error message goes to stderr and is thus > separated from the ``echo "$0: WARNING:..." stuff. I put them together > into a single well-ordered output. Also, "is missing" is not possible.
Hi Bruce, I now see that your patch did not discard stderr, as I said. Rather, it combined stderr with the potentially mangled output from a failing "cat" command, and then printed that potentially tainted output as a suffix of your new diagnostic. If the offending file is a block of NUL bytes or contains some binary garbage, it's better not to print its contents as part of a diagnostic: e.g., if it contains a \r byte before the first newline, it's will appear to erase the diagnostic in some cases. Then there may be terminal escape codes... Also, while it's not likely that the file is missing at that point, it is possible, since there is a race condition.
