[Toybox] [PATCH] Fix od output format when using only legacy type specifiers

[Samuel Holland]

Toybox had been adding the default "oS" output format unless an explicit
-t option had been given, even if one of the legacy shortcut options was
given.

This behavior broke building busybox with CONFIG_FEATURE_COMPRESS_USAGE
as it uses "od -v -b" in applets/usage_compressed
---
toys/posix/od.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/toys/posix/od.c b/toys/posix/od.c
index 6c7cc94..10fcc39 100644
--- a/toys/posix/od.c
+++ b/toys/posix/od.c
@@ -278,7 +278,8 @@ void od_main(void)
if (toys.optflags & FLAG_o) append_base("o2");
if (toys.optflags & FLAG_s) append_base("d2");
if (toys.optflags & FLAG_x) append_base("x2");
- if (!TT.output_base) append_base("o2");
+ if (!TT.output_base && !(toys.optflags &
+ (FLAG_b|FLAG_c|FLAG_d|FLAG_o|FLAG_s|FLAG_x))) append_base("o2");

loopfiles(toys.optargs, do_od);

[Rob Landley]

Post by Samuel Holland
Toybox had been adding the default "oS" output format unless an explicit
-t option had been given, even if one of the legacy shortcut options was
given.

Oops.

Post by Samuel Holland
This behavior broke building busybox with CONFIG_FEATURE_COMPRESS_USAGE
as it uses "od -v -b" in applets/usage_compressed

Busybox defconfig won't build with busybox's tr command (the libm
probing fails), so I'm not _too_ embarassed about that. Still need to
fix it, though. :)

Post by Samuel Holland
- if (!TT.output_base) append_base("o2");
+ if (!TT.output_base && !(toys.optflags &
+ (FLAG_b|FLAG_c|FLAG_d|FLAG_o|FLAG_s|FLAG_x)))
append_base("o2");

Hmmm, that's a much more complicated test, and it's essentially
repeating the flag list right above it.

The TT.output_base check was _supposed_ to cover this. The intended
logic was "only add an output type if we haven't already got an output
type", but output_base is the explicit list of command line output types
so it's checking the wrong variable. TT.types is the count of output
types (incremented by append_base) so checking that's not zero is the
correct thing to do.

Checked in the smaller fix.

Thanks,

Rob

[Samuel Holland]

Post by Rob Landley

Post by Samuel Holland
This behavior broke building busybox with CONFIG_FEATURE_COMPRESS_USAGE
as it uses "od -v -b" in applets/usage_compressed

Busybox defconfig won't build with busybox's tr command (the libm
probing fails), so I'm not _too_ embarassed about that. Still need to
fix it, though. :)

I'm assuming you're referring to building against uClibc. Bootstrapping
busybox works fine against musl, but that's probably because it doesn't
have a separate libm.

Post by Rob Landley

Post by Samuel Holland
- if (!TT.output_base) append_base("o2");
+ if (!TT.output_base && !(toys.optflags &
+ (FLAG_b|FLAG_c|FLAG_d|FLAG_o|FLAG_s|FLAG_x)))
append_base("o2");

Hmmm, that's a much more complicated test, and it's essentially
repeating the flag list right above it.
The TT.output_base check was _supposed_ to cover this. The intended
logic was "only add an output type if we haven't already got an output
type", but output_base is the explicit list of command line output types
so it's checking the wrong variable. TT.types is the count of output
types (incremented by append_base) so checking that's not zero is the
correct thing to do.
Checked in the smaller fix.

Works for me.

Post by Rob Landley
Thanks,
Rob

Thanks,

Samuel

[Samuel Holland]

Post by Rob Landley

Post by Samuel Holland
This behavior broke building busybox with CONFIG_FEATURE_COMPRESS_USAGE
as it uses "od -v -b" in applets/usage_compressed

Busybox defconfig won't build with busybox's tr command (the libm
probing fails), so I'm not _too_ embarassed about that. Still need to
fix it, though. :)

I'm assuming you're referring to building against uClibc. Bootstrapping
busybox works fine against musl, but that's probably because it doesn't
have a separate libm.

Post by Rob Landley

Post by Samuel Holland
- if (!TT.output_base) append_base("o2");
+ if (!TT.output_base && !(toys.optflags &
+ (FLAG_b|FLAG_c|FLAG_d|FLAG_o|FLAG_s|FLAG_x)))
append_base("o2");

Hmmm, that's a much more complicated test, and it's essentially
repeating the flag list right above it.
The TT.output_base check was _supposed_ to cover this. The intended
logic was "only add an output type if we haven't already got an output
type", but output_base is the explicit list of command line output types
so it's checking the wrong variable. TT.types is the count of output
types (incremented by append_base) so checking that's not zero is the
correct thing to do.
Checked in the smaller fix.

Works for me.

Post by Rob Landley
Thanks,
Rob

Thanks,

Samuel

[Samuel Holland]

Toybox had been adding the default "oS" output format unless an explicit
-t option had been given, even if one of the legacy shortcut options was
given.

This behavior broke building busybox with CONFIG_FEATURE_COMPRESS_USAGE
as it uses "od -v -b" in applets/usage_compressed
---
toys/posix/od.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/toys/posix/od.c b/toys/posix/od.c
index 6c7cc94..10fcc39 100644
--- a/toys/posix/od.c
+++ b/toys/posix/od.c
@@ -278,7 +278,8 @@ void od_main(void)
if (toys.optflags & FLAG_o) append_base("o2");
if (toys.optflags & FLAG_s) append_base("d2");
if (toys.optflags & FLAG_x) append_base("x2");
- if (!TT.output_base) append_base("o2");
+ if (!TT.output_base && !(toys.optflags &
+ (FLAG_b|FLAG_c|FLAG_d|FLAG_o|FLAG_s|FLAG_x))) append_base("o2");

loopfiles(toys.optargs, do_od);

[Rob Landley]

Post by Samuel Holland
Toybox had been adding the default "oS" output format unless an explicit
-t option had been given, even if one of the legacy shortcut options was
given.

Oops.

Post by Samuel Holland
This behavior broke building busybox with CONFIG_FEATURE_COMPRESS_USAGE
as it uses "od -v -b" in applets/usage_compressed

Busybox defconfig won't build with busybox's tr command (the libm
probing fails), so I'm not _too_ embarassed about that. Still need to
fix it, though. :)

Post by Samuel Holland
- if (!TT.output_base) append_base("o2");
+ if (!TT.output_base && !(toys.optflags &
+ (FLAG_b|FLAG_c|FLAG_d|FLAG_o|FLAG_s|FLAG_x)))
append_base("o2");

Hmmm, that's a much more complicated test, and it's essentially
repeating the flag list right above it.

The TT.output_base check was _supposed_ to cover this. The intended
logic was "only add an output type if we haven't already got an output
type", but output_base is the explicit list of command line output types
so it's checking the wrong variable. TT.types is the count of output
types (incremented by append_base) so checking that's not zero is the
correct thing to do.

Checked in the smaller fix.

Thanks,

Rob