On Wed, Dec 21, 2016 at 10:54:31PM +0100, Heinrich Schuchardt wrote:
> A new command line option is added
> -c, --color <arg> Colorize, argument can be auto, never or always
> which defaults to auto.
>
> For colored output in pipes use 'always', e.g.
> fdtdump -c always file.dtb | less -R
>
> If you don't like colors use
> alias fdtdump="fdtdump -c none"
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@xxxxxx>
Hmm. So, as I said, I really see fdtdump as a minimal hack. For real
life use I recommend dtc -I dtb -O dts instead. So, I'm not thrilled
about adding a feature like this to fdtdump but not to dtc's dts
output.
> ---
> Makefile | 2 +-
> fdtdump.c | 37 +++++++++++++++++++++++++++++++++----
> util.c | 12 +++++++-----
> util.h | 8 ++++++++
> 4 files changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 32dcfcf..ee543b9 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -18,7 +18,7 @@ CONFIG_LOCALVERSION =
> CPPFLAGS = -I libfdt -I .
> WARNINGS = -Wall -Wpointer-arith -Wcast-qual -Wnested-externs \
> -Wstrict-prototypes -Wmissing-prototypes -Wredundant-decls -Wshadow
> -CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
> +CFLAGS = -g -Os -fPIC -Werror $(WARNINGS) -lncurses
>
> BISON = bison
> LEX = flex
> diff --git a/fdtdump.c b/fdtdump.c
> index 717fef5..59194cd 100644
> --- a/fdtdump.c
> +++ b/fdtdump.c
> @@ -2,6 +2,9 @@
> * fdtdump.c - Contributed by Pantelis Antoniou <pantelis.antoniou AT gmail.com>
> */
>
> +#include <unistd.h>
> +#include <curses.h>
> +#include <term.h>
> #include <stdbool.h>
> #include <stdint.h>
> #include <stdio.h>
> @@ -63,7 +66,7 @@ static void dump_blob(void *blob, bool debug)
> depth = 0;
> shift = 4;
>
> - printf("/dts-v1/;\n");
> + printf("%s/dts-v1/;\n", COLNONE);
> printf("// magic:\t\t0x%x\n", fdt32_to_cpu(bph->magic));
> printf("// totalsize:\t\t0x%x (%d)\n", totalsize, totalsize);
> printf("// off_dt_struct:\t0x%x\n", off_dt);
> @@ -104,10 +107,11 @@ static void dump_blob(void *blob, bool debug)
> s = p;
> p = PALIGN(p + strlen(s) + 1, 4);
>
> + printf("%s", COLNODE);
> if (*s == '\0')
> s = "/";
>
> - printf("%*s%s {\n", depth * shift, "", s);
> + printf("%*s%s%s {\n", depth * shift, "", s, COLNONE);
>
> depth++;
> continue;
> @@ -139,7 +143,7 @@ static void dump_blob(void *blob, bool debug)
>
> dumpf("%04zx: string: %s\n", (uintptr_t)s - blob_off, s);
> dumpf("%04zx: value\n", (uintptr_t)t - blob_off);
> - printf("%*s%s", depth * shift, "", s);
> + printf("%*s%s%s%s", depth * shift, "", COLPROP, s, COLNONE);
> utilfdt_print_data(t, sz);
> printf(";\n");
> }
> @@ -147,18 +151,33 @@ static void dump_blob(void *blob, bool debug)
>
> /* Usage related data. */
> static const char usage_synopsis[] = "fdtdump [options] <file>";
> -static const char usage_short_opts[] = "ds" USAGE_COMMON_SHORT_OPTS;
> +static const char usage_short_opts[] = "c:ds" USAGE_COMMON_SHORT_OPTS;
> static struct option const usage_long_opts[] = {
> + {"color", a_argument, NULL, 'c'},
> {"debug", no_argument, NULL, 'd'},
> {"scan", no_argument, NULL, 's'},
> USAGE_COMMON_LONG_OPTS
> };
> static const char * const usage_opts_help[] = {
> + "Colorize, argument can be auto, never or always",
> "Dump debug information while decoding the file",
> "Scan for an embedded fdt in file",
> USAGE_COMMON_OPTS_HELP
> };
>
> +static void check_colored(void) {
> + int ret;
> +
> + colored = 0;
> + if (isatty(STDOUT_FILENO) != 1)
> + return;
> + if (setupterm(NULL, STDOUT_FILENO, &ret) != OK || ret != 1)
> + return;
> + if (2 > tigetnum("colors"))
> + return;
> + colored = 1;
> +}
> +
> int main(int argc, char *argv[])
> {
> int opt;
> @@ -168,10 +187,20 @@ int main(int argc, char *argv[])
> bool scan = false;
> off_t len;
>
> + check_colored();
> +
> while ((opt = util_getopt_long()) != EOF) {
> switch (opt) {
> case_USAGE_COMMON_FLAGS
>
> + case 'c':
> + if (!strcmp("none", optarg))
> + colored = 0;
> + else if (!strcmp("always", optarg))
> + colored = 1;
> + else if (strcmp("auto", optarg))
> + usage("invalid argument for colored");
> + break;
> case 'd':
> debug = true;
> break;
> diff --git a/util.c b/util.c
> index 3550f86..29fd6ad 100644
> --- a/util.c
> +++ b/util.c
> @@ -36,6 +36,8 @@
> #include "util.h"
> #include "version_gen.h"
>
> +int colored = 0;
> +
> char *xstrdup(const char *s)
> {
> int len = strlen(s) + 1;
> @@ -389,7 +391,7 @@ void utilfdt_print_data(const char *data, int len)
>
> s = data;
> do {
> - printf("\"%s\"", s);
> + printf("\"%s%s%s\"", COLSTRG, s, COLNONE);
> s += strlen(s) + 1;
> if (s < data + len)
> printf(", ");
> @@ -398,17 +400,17 @@ void utilfdt_print_data(const char *data, int len)
> } else if ((len % 4) == 0) {
> const uint32_t *cell = (const uint32_t *)data;
>
> - printf(" = <");
> + printf(" = <%s", COLNUMB);
> for (i = 0, len /= 4; i < len; i++)
> printf("0x%08x%s", fdt32_to_cpu(cell[i]),
> i < (len - 1) ? " " : "");
> - printf(">");
> + printf("%s>", COLNONE);
> } else {
> const unsigned char *p = (const unsigned char *)data;
> - printf(" = [");
> + printf(" = [%s", COLBYTE);
> for (i = 0; i < len; i++)
> printf("%02x%s", *p++, i < len - 1 ? " " : "");
> - printf("]");
> + printf("%s]", COLNONE);
> }
> }
>
> diff --git a/util.h b/util.h
> index f5c4f1b..b814872 100644
> --- a/util.h
> +++ b/util.h
> @@ -25,6 +25,14 @@
> * USA
> */
>
> +extern int colored;
> +#define COLNODE (colored ? "\x1b[35m" : "") /* magenta */
> +#define COLPROP (colored ? "\x1b[32m" : "") /* green */
> +#define COLSTRG (colored ? "\x1b[33m" : "") /* yellow */
> +#define COLNUMB (colored ? "\x1b[34m" : "") /* blue */
> +#define COLBYTE (colored ? "\x1b[36m" : "") /* cyan */
> +#define COLNONE (colored ? "\x1b[0m" : "") /* default */
Um.. this looks absolutely wrong. You've gone to the trouble of
including the ncurses library, but you're basically not using it
except for the inital on/off check. Here you've used fixed explicit
escape codes, which will only be correct for one terminal type -
terminfo and ncurses basically exist for abstracting this sort of thing.
> #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>
> static inline void __attribute__((noreturn)) die(const char *str, ...)
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
