[Toybox] [landley/toybox] xxd -i: not expected behaviour (#286)

[Rob Landley]

cc-ing the toybox mailing list on this reply. And I also pinged
http://lists.busybox.net/pipermail/busybox/2021-June/088931.html because it's
just too on the nose not too. (When you're right, you're right...)

On 6/24/21 11:21 AM, enh-google wrote:
> annoyingly, the default vim xxd output isn't ideal for C either --- the use
> of int rather than size_t causes warnings, and there's no easy way to mark
> both variables static or const or visibility hidden or...

The existence of xxd -C and -u to micromanage the -i output seems kind of
awkward as well. (And xxd -E is just inexcusable.)

What you've implemented is compatible with python and java and so on. Heck,
probably fortran. CSV is a lingua franca. Heck, bash can grok it:

$ toybox xxd -i README | while read -d, i; do printf \\${i:1}; done

It's definitely the way they SHOULD have done it in the first place, but
unfortunately they didn't. The problem is compatibility with existing tools that
do extra stupid stuff by default...

> looking at the callers in Android that seem to rely on the current
> behavior, it doesn't look like many (4?), and since it's clearly my fault
> i'm happy to own cleaning up the mess.

Yay cleaning.

I lean towards adding a capital version of the existing option (-I) that skips
the header/trailer stuff, and we can even poke the vim guys about it to make
Denys happy. :)

The "compatible" alternative is to add --gratuitous-longopt-with-no-short which
is less of a contested namespace. Of course it still breaks EXACTLY the same way
if you run AOSP with the vim tool instead of the toybox one, and by that logic
nobody would ever add another short option again.

> how does making `xxd -i` behave like vim xxd, but adding a flag something
> like --bare to not include any boilerplate? (since although we could add
> flags for golang or whatever alongside one for C,

Oh please no. It's trivial to wrap the xxd output with header/footer lines, and
that's the "unix" way of doing things. Simple tools that connect together with
pipelines. As much of a fan of lua as I am, I'm not adding explicit support for
it to xxd.

The c output is grandfathered in, hysterical raisins etc. We can poke vim (and
again, busybox) to add the new "bare" output type.

> like i said: there's no
> One True Answer for C anyway,

It wasn't xxd's job to get C output syntax right in the first place, trying to
get it right for MORE languages is not going to decrease the entropic attack
surface.

> so i'd assume other languages have equivalent
> problems too. and even if they don't have _those_ problems, anecdotally i'd
> say _most_ callers don't like the way vim xxd chooses a name for the
> variables, and sed their own in after the fact anyway![1])

Indeed. I vote for "xxd -I" doing the unwrapped version but also recuse myself
from actual decision making here because I'm not a regular user of this tool and
Elliott is (and he has an existing userbase of scripts to migrate, the only
regression tests I have are artificial ones in tests/xxd.test which prove
nothing in this sort of circumstance).

> if we do go this way, i'll probably try to get in a no-op --bare before i
> change -i, because that'll make the transition easier for me.

How about we add -I as a synonym for -i and then switch toybox over to have -i
produce the conventional C output in a couple weeks?

I expect Denys is going to make us contact vim _before_ busybox about the new
option this time, but I'm personally ok with that because vim isn't a gnu
project nor is it GPLv3. (Heck, its developer says on his web page he can be
outright bought: https://www.vim.org/sponsor/index.php .)

Although I expect "sending him a patch" is likely to work. :)

> only two of
> them seem to be used as part of the build anyway --- people seem to be
> writing scripts but checking in the [hand-modified] output :-(

Do I have a https://landley.net/toybox/cleanup.html#advice link about not
checking in generated files? No? I'm sure I've ranted about it before. Possibly
in my blog.

I was in favor of it as the lesser evil when the linux kernel used it to avoid
having kconfig depend on lex, and toybox has kconfig/*_shipped files inherited
from that. But that's ALSO another reason to reimplement the kconfig directory
from scratch.

Rob