Re: [PATCH 1/3] fast-export: allow dumping the refname mapping

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

 



On Fri, Jun 19, 2020 at 02:00:47PM -0400, Eric Sunshine wrote:

> > so the whitespace is eaten not when "wc" is run, but rather when the
> > variable is expanded.
> 
> Not something that should be done by this series (more a
> left-over-bitty thing, perhaps), but this almost suggests that
> test_line_count() deserves a tweak to make it more robust against that
> sort of thing:
> 
>     test_line_count () {
>         if test $# != 3
>         then
>             BUG "not 3 parameters to test_line_count"
>         elif ! test $(wc -l <"$3") "$1" "$2"
>         then
>             echo "test_line_count: line count for $3 !$1 $2"
>             cat "$3"
>             return 1
>         fi
>     }
> 
> If we drop the quotes around $2 from the 'test':
> 
>     elif ! test $(wc -l <"$3") "$1" $2
> 
> then your code would have worked as expected.
> 
> My only worry about that is that a poorly written caller would get a
> weird and unhelpful error message:
> 
>     test_line_count = 4 4
>     --> sh: test: too many arguments

I think your unhelpful-error-message case would happen only if the
length argument contains two non-whitespace tokens separated by a
whitespace (so the shell splits them into two arguments), _and_ the
caller passed that argument in quotes (otherwise the shell would split
it at the function call and we'd hit the BUG message). In which case,
what are they trying to do with passing "4 4" to "test"? And since we're
using "=" and not "-eq", I think "test" would be complaining about that.

So it seems like it might be a reasonable change to make things more
friendly.

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux