Re: [PATCH 1/2] test-lib-functions: make packetize() more efficient

From: Jeff King <peff@peff.net>
Date: Mar 28, 2020, 12:25 UTC
Message-ID: <20200328122534.GA642919@coredump.intra.peff.net>
In-Reply-To: <20200327151607.GA30419@syl.local>

On Fri, Mar 27, 2020 at 09:16:07AM -0600, Taylor Blau wrote:

Show 19 quoted lines

> >  packetize() {
> > -	cat >packetize.tmp &&
> > -	len=$(wc -c <packetize.tmp) &&
> > -	printf '%04x%s' "$(($len + 4))" &&
> > -	cat packetize.tmp &&
> > -	rm -f packetize.tmp
> > +	if test $# -gt 0
> > +	then
> > +		packet="$*"
> 
> Mentioned off-list in a discussion already, but: though I find this
> behavior of joining multiple arguments by a whitespace character a
> little confusing (i.e., what would callers thing this function does if
> they hadn't read the documentation?) I think that this is probably the
> most sane thing that you could do here.
> 
> On the other hand, nowhere in this patch do we currently call packetize
> with multiple arguments, so perhaps it would be made simpler if we
> instead wrote "$1" anywhere there was "$packet".

Of all the options, I like that the least because somebody doing:

  packetize foo bar

would have the "bar" silently ignored. And it's more lines of code to check and complain about that than it is to just do something sensible.

-Peff