[PATCH 00/10 v3] usage()/--help clean up & unification, and extend fdtdump

[PATCH 00/10 v3] usage()/--help clean up & unification, and extend fdtdump

[Mike Frysinger]

Each utility currently open codes how they process options and
implement usage().  This leads to inconsistent behavior and output
which sucks.  A bunch also get common things wrong (like sending
--help to stderr).

Rather than go through them one by one and fix their bugs, start
a new mini framework in the util.[ch] code base.  This allows the
utils to be much more data driven in their approach to options --
they declare certain structures with the right format, and then
call usage() as needed.  The new util code takes care of the rest.

I also snuck in a few minor fixes in this patch series that I
noticed as I was hacking on things.  Really I just wanted --scan
and --debug options in the fdtdump tool :).

I've made sure each patch builds & passes `make check` by itself.
So things should be git bisectable.

For the v3 series:
 - add dedicated _len read helpers rather than rework existing ones
 - rename dprintf() to dumpf() in fdtdump

Mike Frysinger (10):
  utilfdt_read_err: use xmalloc funcs
  utilfdt_read: pass back up the length of data read
  die: constify format string arg
  util_version: new helper for displaying version info
  fdtdump: make usage a bit more friendly
  fdtdump: add a --scan option
  dtc/fdt{get,put}/convert-dtsv0-lexer: convert to new usage helpers
  util: drop "long" from usage helpers
  util: add common ARRAY_SIZE define
  fdtdump: add a debug mode

 convert-dtsv0-lexer.l |  24 +++++++----
 dtc.c                 | 116 +++++++++++++++++++++++++-------------------------
 dtc.h                 |   1 -
 fdtdump.c             | 110 ++++++++++++++++++++++++++++++++++++++++++-----
 fdtget.c              |  60 ++++++++++++--------------
 fdtput.c              |  63 +++++++++++++--------------
 util.c                |  84 +++++++++++++++++++++++++++++++++---
 util.h                |  85 +++++++++++++++++++++++++++++++++++-
 8 files changed, 393 insertions(+), 150 deletions(-)

-- 
1.8.1.2

[PATCH 01/10] utilfdt_read_err: use xmalloc funcs

[Mike Frysinger]

We've got these handy helpers, so let's use them.

Acked-by: David Gibson <David-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 util.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/util.c b/util.c
index b081fa8..12f0614 100644
--- a/util.c
+++ b/util.c
@@ -212,12 +212,12 @@ int utilfdt_read_err(const char *filename, char **buffp)
 	}
 
 	/* Loop until we have read everything */
-	buf = malloc(bufsize);
+	buf = xmalloc(bufsize);
 	do {
 		/* Expand the buffer to hold the next chunk */
 		if (offset == bufsize) {
 			bufsize *= 2;
-			buf = realloc(buf, bufsize);
+			buf = xrealloc(buf, bufsize);
 			if (!buf) {
 				ret = ENOMEM;
 				break;
-- 
1.8.1.2

[PATCH 02/10] utilfdt_read: pass back up the length of data read

[Mike Frysinger]

For a follow up commit, we want to be able to scan the buffer that was
returned to us.  In order to do that safely, we need to know how big
the buffer actually is, so create a new set of funcs to pass that back.

Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 util.c | 19 ++++++++++++++++---
 util.h | 13 +++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/util.c b/util.c
index 12f0614..0367df6 100644
--- a/util.c
+++ b/util.c
@@ -197,7 +197,7 @@ char get_escape_char(const char *s, int *i)
 	return val;
 }
 
-int utilfdt_read_err(const char *filename, char **buffp)
+int utilfdt_read_err_len(const char *filename, char **buffp, off_t *len)
 {
 	int fd = 0;	/* assume stdin */
 	char *buf = NULL;
@@ -238,13 +238,20 @@ int utilfdt_read_err(const char *filename, char **buffp)
 		free(buf);
 	else
 		*buffp = buf;
+	*len = bufsize;
 	return ret;
 }
 
-char *utilfdt_read(const char *filename)
+int utilfdt_read_err(const char *filename, char **buffp)
+{
+	off_t len;
+	return utilfdt_read_err_len(filename, buffp, &len);
+}
+
+char *utilfdt_read_len(const char *filename, off_t *len)
 {
 	char *buff;
-	int ret = utilfdt_read_err(filename, &buff);
+	int ret = utilfdt_read_err_len(filename, &buff, len);
 
 	if (ret) {
 		fprintf(stderr, "Couldn't open blob from '%s': %s\n", filename,
@@ -255,6 +262,12 @@ char *utilfdt_read(const char *filename)
 	return buff;
 }
 
+char *utilfdt_read(const char *filename)
+{
+	off_t len;
+	return utilfdt_read_len(filename, &len);
+}
+
 int utilfdt_write_err(const char *filename, const void *blob)
 {
 	int fd = 1;	/* assume stdout */
diff --git a/util.h b/util.h
index 543a173..1f0ef47 100644
--- a/util.h
+++ b/util.h
@@ -85,6 +85,13 @@ char get_escape_char(const char *s, int *i);
 char *utilfdt_read(const char *filename);
 
 /**
+ * Like utilfdt_read(), but also passes back the size of the file read.
+ *
+ * @param len		If non-NULL, the amount of data we managed to read
+ */
+char *utilfdt_read_len(const char *filename, off_t *len);
+
+/**
  * Read a device tree file into a buffer. Does not report errors, but only
  * returns them. The value returned can be passed to strerror() to obtain
  * an error message for the user.
@@ -95,6 +102,12 @@ char *utilfdt_read(const char *filename);
  */
 int utilfdt_read_err(const char *filename, char **buffp);
 
+/**
+ * Like utilfdt_read_err(), but also passes back the size of the file read.
+ *
+ * @param len		If non-NULL, the amount of data we managed to read
+ */
+int utilfdt_read_err_len(const char *filename, char **buffp, off_t *len);
 
 /**
  * Write a device tree buffer to a file. This will report any errors on
-- 
1.8.1.2

Re: [PATCH 02/10] utilfdt_read: pass back up the length of data read

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 650 bytes --]

On Mon, Apr 15, 2013 at 10:13:09PM -0400, Mike Frysinger wrote:
> For a follow up commit, we want to be able to scan the buffer that was
> returned to us.  In order to do that safely, we need to know how big
> the buffer actually is, so create a new set of funcs to pass that back.
> 
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 03/10] die: constify format string arg

[Mike Frysinger]

We only display this string, so there's no need for it to be writable.
Constify away!

Acked-by: David Gibson <David-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 util.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util.h b/util.h
index 1f0ef47..b9c6d7e 100644
--- a/util.h
+++ b/util.h
@@ -23,7 +23,7 @@
  *                                                                   USA
  */
 
-static inline void __attribute__((noreturn)) die(char * str, ...)
+static inline void __attribute__((noreturn)) die(const char *str, ...)
 {
 	va_list ap;
 
-- 
1.8.1.2

[PATCH 04/10] util_version: new helper for displaying version info

[Mike Frysinger]

This is so all utilities can have this flag and not just dtc.

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 dtc.c  | 5 +----
 util.c | 7 +++++++
 util.h | 5 +++++
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/dtc.c b/dtc.c
index d40e220..e4e1b84 100644
--- a/dtc.c
+++ b/dtc.c
@@ -21,8 +21,6 @@
 #include "dtc.h"
 #include "srcpos.h"
 
-#include "version_gen.h"
-
 /*
  * Command line options
  */
@@ -158,8 +156,7 @@ int main(int argc, char *argv[])
 			srcfile_add_search_path(optarg);
 			break;
 		case 'v':
-			printf("Version: %s\n", DTC_VERSION);
-			exit(0);
+			util_version();
 		case 'H':
 			if (streq(optarg, "legacy"))
 				phandle_format = PHANDLE_LEGACY;
diff --git a/util.c b/util.c
index 0367df6..350cf8b 100644
--- a/util.c
+++ b/util.c
@@ -34,6 +34,7 @@
 
 #include "libfdt.h"
 #include "util.h"
+#include "version_gen.h"
 
 char *xstrdup(const char *s)
 {
@@ -385,3 +386,9 @@ void utilfdt_print_data(const char *data, int len)
 		printf("]");
 	}
 }
+
+void util_version(void)
+{
+	printf("Version: %s\n", DTC_VERSION);
+	exit(0);
+}
diff --git a/util.h b/util.h
index b9c6d7e..95ae531 100644
--- a/util.h
+++ b/util.h
@@ -179,4 +179,9 @@ int utilfdt_decode_type(const char *fmt, int *type, int *size);
  */
 void utilfdt_print_data(const char *data, int len);
 
+/**
+ * Show source version and exit
+ */
+void util_version(void) __attribute__((noreturn));
+
 #endif /* _UTIL_H */
-- 
1.8.1.2

Re: [PATCH 04/10] util_version: new helper for displaying version info

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 582 bytes --]

On Mon, Apr 15, 2013 at 10:13:11PM -0400, Mike Frysinger wrote:
> This is so all utilities can have this flag and not just dtc.
> 
> Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>

Jon,

Please apply these first 4 patches, regardless of the rest of the
series.

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 04/10] util_version: new helper for displaying version info

[Jon Loeliger]

> 
> On Mon, Apr 15, 2013 at 10:13:11PM -0400, Mike Frysinger wrote:
> > This is so all utilities can have this flag and not just dtc.
> >=20
> > Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> 
> Jon,
> 
> Please apply these first 4 patches, regardless of the rest of the
> series.

Done.

Thanks,
jdl

Re: [PATCH 04/10] util_version: new helper for displaying version info

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 828 bytes --]

On Sunday 21 April 2013 15:26:03 Jon Loeliger wrote:
> > On Mon, Apr 15, 2013 at 10:13:11PM -0400, Mike Frysinger wrote:
> > > This is so all utilities can have this flag and not just dtc.
> > >
> > >=20
> > >
> > > Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > > Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> > 
> > Jon,
> > 
> > Please apply these first 4 patches, regardless of the rest of the
> > series.
> 
> Done.

hmm, looks like patch #2 (utilfdt_read and handling of the len argument) was 
taken from the first patch series rather than the 3rd ?

i'd suggest reverting it and applying instead the one from this series ...
Message-Id: <1366078397-14889-3-git-send-email-vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 04/10] util_version: new helper for displaying version info

[Jon Loeliger]

> 
> 
> hmm, looks like patch #2 (utilfdt_read and handling of the len argument) was
> taken from the first patch series rather than the 3rd ?
> 
> i'd suggest reverting it and applying instead the one from this series ...
> Message-Id: <1366078397-14889-3-git-send-email-vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>

Please send me the version you want applied.

jdl

Re: [PATCH 04/10] util_version: new helper for displaying version info

[Mike Frysinger]

[-- Attachment #1.1.1: Type: Text/Plain, Size: 454 bytes --]

On Monday 22 April 2013 11:37:12 Jon Loeliger wrote:
> > hmm, looks like patch #2 (utilfdt_read and handling of the len argument)
> > was taken from the first patch series rather than the 3rd ?
> > 
> > i'd suggest reverting it and applying instead the one from this series
> > ... Message-Id: <1366078397-14889-3-git-send-email-vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> 
> Please send me the version you want applied.

attached
-mike

[-- Attachment #1.1.2: 0001-utilfdt_read-pass-back-up-the-length-of-data-read.patch --]
[-- Type: text/x-patch, Size: 3122 bytes --]

From 7a0e8d370b62b902ccf3defb41e9af2d3d34e177 Mon Sep 17 00:00:00 2001
From: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
Date: Mon, 8 Apr 2013 00:56:54 -0400
Subject: [PATCH] utilfdt_read: pass back up the length of data read

For a follow up commit, we want to be able to scan the buffer that was
returned to us.  In order to do that safely, we need to know how big
the buffer actually is, so create a new set of funcs to pass that back.

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 util.c | 19 ++++++++++++++++---
 util.h | 13 +++++++++++++
 2 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/util.c b/util.c
index 12f0614..0367df6 100644
--- a/util.c
+++ b/util.c
@@ -197,7 +197,7 @@ char get_escape_char(const char *s, int *i)
 	return val;
 }
 
-int utilfdt_read_err(const char *filename, char **buffp)
+int utilfdt_read_err_len(const char *filename, char **buffp, off_t *len)
 {
 	int fd = 0;	/* assume stdin */
 	char *buf = NULL;
@@ -238,13 +238,20 @@ int utilfdt_read_err(const char *filename, char **buffp)
 		free(buf);
 	else
 		*buffp = buf;
+	*len = bufsize;
 	return ret;
 }
 
-char *utilfdt_read(const char *filename)
+int utilfdt_read_err(const char *filename, char **buffp)
+{
+	off_t len;
+	return utilfdt_read_err_len(filename, buffp, &len);
+}
+
+char *utilfdt_read_len(const char *filename, off_t *len)
 {
 	char *buff;
-	int ret = utilfdt_read_err(filename, &buff);
+	int ret = utilfdt_read_err_len(filename, &buff, len);
 
 	if (ret) {
 		fprintf(stderr, "Couldn't open blob from '%s': %s\n", filename,
@@ -255,6 +262,12 @@ char *utilfdt_read(const char *filename)
 	return buff;
 }
 
+char *utilfdt_read(const char *filename)
+{
+	off_t len;
+	return utilfdt_read_len(filename, &len);
+}
+
 int utilfdt_write_err(const char *filename, const void *blob)
 {
 	int fd = 1;	/* assume stdout */
diff --git a/util.h b/util.h
index 543a173..1f0ef47 100644
--- a/util.h
+++ b/util.h
@@ -85,6 +85,13 @@ char get_escape_char(const char *s, int *i);
 char *utilfdt_read(const char *filename);
 
 /**
+ * Like utilfdt_read(), but also passes back the size of the file read.
+ *
+ * @param len		If non-NULL, the amount of data we managed to read
+ */
+char *utilfdt_read_len(const char *filename, off_t *len);
+
+/**
  * Read a device tree file into a buffer. Does not report errors, but only
  * returns them. The value returned can be passed to strerror() to obtain
  * an error message for the user.
@@ -95,6 +102,12 @@ char *utilfdt_read(const char *filename);
  */
 int utilfdt_read_err(const char *filename, char **buffp);
 
+/**
+ * Like utilfdt_read_err(), but also passes back the size of the file read.
+ *
+ * @param len		If non-NULL, the amount of data we managed to read
+ */
+int utilfdt_read_err_len(const char *filename, char **buffp, off_t *len);
 
 /**
  * Write a device tree buffer to a file. This will report any errors on
-- 
1.8.2.1


[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 04/10] util_version: new helper for displaying version info

[Jon Loeliger]

> On Monday 22 April 2013 11:37:12 Jon Loeliger wrote:
> > > hmm, looks like patch #2 (utilfdt_read and handling of the len argument)
> > > was taken from the first patch series rather than the 3rd ?
> > >
> > > i'd suggest reverting it and applying instead the one from this series
> > > ... Message-Id: <1366078397-14889-3-git-send-email-vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> >
> > Please send me the version you want applied.
> 
> attached
> Dmike

...and re-applied.  Sorry about that.

a6d55e0 utilfdt_read: pass back up the length of data read
5543b88 Revert "utilfdt_read: pass back up the length of data read"
31be4ce util_version: new helper for displaying version info
97c122e die: constify format string arg
cc2c178 utilfdt_read: pass back up the length of data read
f8cb5dd utilfdt_read_err: use xmalloc funcs
27cdc1b Added license header to dtc/libfdt/fdt.h and libfdt_env.h
cc11e52 Fix typo
b7aa300 Export fdt_stringlist_contains()
d59b807 .gitignore: Add rule for *.patch

Re: [PATCH 04/10] util_version: new helper for displaying version info

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 641 bytes --]

On Monday 22 April 2013 16:44:54 Jon Loeliger wrote:
> > On Monday 22 April 2013 11:37:12 Jon Loeliger wrote:
> > > > hmm, looks like patch #2 (utilfdt_read and handling of the len
> > > > argument) was taken from the first patch series rather than the 3rd
> > > > ?
> > > > 
> > > > i'd suggest reverting it and applying instead the one from this
> > > > series ... Message-Id:
> > > > <1366078397-14889-3-git-send-email-vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> > > 
> > > Please send me the version you want applied.
> > 
> > attached
> 
> ...and re-applied.  Sorry about that.

np.  things look good now.
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 04/10] util_version: new helper for displaying version info

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 946 bytes --]

On Thursday 18 April 2013 20:50:46 David Gibson wrote:
> On Mon, Apr 15, 2013 at 10:13:11PM -0400, Mike Frysinger wrote:
> > This is so all utilities can have this flag and not just dtc.
> > 
> > Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> 
> Please apply these first 4 patches, regardless of the rest of the
> series.

on the subject of the other patches, is there a way i could split them up 
that'd make things more palatable ?  the usage clean up ones could be merged 
independently of the new fdtdump options.

clean up series:
fdtdump: make usage a bit more friendly
dtc/fdt{get,put}/convert-dtsv0-lexer: convert to new usage helpers
util: drop "long" from usage helpers
util: add common ARRAY_SIZE define

new functionality series:
fdtdump: add a --scan option
fdtdump: add a debug mode
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 04/10] util_version: new helper for displaying version info

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 1814 bytes --]

On Mon, Apr 22, 2013 at 05:19:41PM -0400, Mike Frysinger wrote:
> On Thursday 18 April 2013 20:50:46 David Gibson wrote:
> > On Mon, Apr 15, 2013 at 10:13:11PM -0400, Mike Frysinger wrote:
> > > This is so all utilities can have this flag and not just dtc.
> > > 
> > > Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> > > Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> > 
> > Please apply these first 4 patches, regardless of the rest of the
> > series.
> 
> on the subject of the other patches, is there a way i could split them up 
> that'd make things more palatable ?  the usage clean up ones could be merged 
> independently of the new fdtdump options.

I don't think there's anything really wrong with the split - it's just
they're a little more involved, and so it's taken me longer to look
through them properly.  In particular I've taken a bid of a sidetrack
working out what is broken for our build on freebsd at the moment and
fixing that to see how these patches affect it.

> clean up series:
> fdtdump: make usage a bit more friendly
> dtc/fdt{get,put}/convert-dtsv0-lexer: convert to new usage helpers
> util: drop "long" from usage helpers
> util: add common ARRAY_SIZE define
> 
> new functionality series:
> fdtdump: add a --scan option
> fdtdump: add a debug mode

I don't think it makes a big difference, and two series kind of
complicates things.  That order is a bit better than the original, at
least if you pull ARRAY_SIZE out to the front (I'd like to see that go
in right now, actually).

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 05/10] fdtdump: make usage a bit more friendly

[Mike Frysinger]

This starts a new usage framework and then cuts fdtdump over to it.
Now we can do `fdtdump -h` and get something useful back.

Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 fdtdump.c | 31 +++++++++++++++++++++++--------
 util.c    | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 util.h    | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 138 insertions(+), 8 deletions(-)

diff --git a/fdtdump.c b/fdtdump.c
index 03ea429..a6e522c 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -117,21 +117,36 @@ static void dump_blob(void *blob)
 	}
 }
 
+/* Usage related data. */
+static const char usage_synopsis[] = "fdtdump [options] <file>";
+static const char usage_short_opts[] = USAGE_COMMON_SHORT_OPTS;
+static struct option const usage_long_opts[] = {
+	USAGE_COMMON_LONG_OPTS
+};
+static const char * const usage_opts_help[] = {
+	USAGE_COMMON_OPTS_HELP
+};
 
 int main(int argc, char *argv[])
 {
+	int opt;
+	const char *file;
 	char *buf;
 
-	if (argc < 2) {
-		fprintf(stderr, "supply input filename\n");
-		return 5;
+	while ((opt = util_getopt_long()) != EOF) {
+		switch (opt) {
+		case_USAGE_COMMON_FLAGS
+		}
 	}
+	if (optind != argc - 1)
+		long_usage("missing input filename");
+	file = argv[optind];
+
+	buf = utilfdt_read(file);
+	if (!buf)
+		die("could not read: %s\n", file);
 
-	buf = utilfdt_read(argv[1]);
-	if (buf)
-		dump_blob(buf);
-	else
-		return 10;
+	dump_blob(buf);
 
 	return 0;
 }
diff --git a/util.c b/util.c
index 350cf8b..d9a823a 100644
--- a/util.c
+++ b/util.c
@@ -392,3 +392,57 @@ void util_version(void)
 	printf("Version: %s\n", DTC_VERSION);
 	exit(0);
 }
+
+void util_long_usage(const char *errmsg, const char *synopsis,
+		     const char *short_opts, struct option const long_opts[],
+		     const char * const opts_help[])
+{
+	FILE *fp = errmsg ? stderr : stdout;
+	const char a_arg[] = "<arg>";
+	size_t a_arg_len = strlen(a_arg) + 1;
+	size_t i;
+	int optlen;
+
+	fprintf(fp,
+		"Usage: %s\n"
+		"\n"
+		"Options: -[%s]\n", synopsis, short_opts);
+
+	/* prescan the --long opt length to auto-align */
+	optlen = 0;
+	for (i = 0; long_opts[i].name; ++i) {
+		/* +1 is for space between --opt and help text */
+		int l = strlen(long_opts[i].name) + 1;
+		if (long_opts[i].has_arg == a_argument)
+			l += a_arg_len;
+		if (optlen < l)
+			optlen = l;
+	}
+
+	for (i = 0; long_opts[i].name; ++i) {
+		/* helps when adding new applets or options */
+		assert(opts_help[i] != NULL);
+
+		/* first output the short flag if it has one */
+		if (long_opts[i].val > '~')
+			fprintf(fp, "      ");
+		else
+			fprintf(fp, "  -%c, ", long_opts[i].val);
+
+		/* then the long flag */
+		if (long_opts[i].has_arg == no_argument)
+			fprintf(fp, "--%-*s", optlen, long_opts[i].name);
+		else
+			fprintf(fp, "--%s %s%*s", long_opts[i].name, a_arg,
+				(int)(optlen - strlen(long_opts[i].name) - a_arg_len), "");
+
+		/* finally the help text */
+		fprintf(fp, "%s\n", opts_help[i]);
+	}
+
+	if (errmsg) {
+		fprintf(fp, "\nError: %s\n", errmsg);
+		exit(EXIT_FAILURE);
+	} else
+		exit(EXIT_SUCCESS);
+}
diff --git a/util.h b/util.h
index 95ae531..1da3bd3 100644
--- a/util.h
+++ b/util.h
@@ -2,6 +2,7 @@
 #define _UTIL_H
 
 #include <stdarg.h>
+#include <getopt.h>
 
 /*
  * Copyright 2011 The Chromium Authors, All Rights Reserved.
@@ -184,4 +185,64 @@ void utilfdt_print_data(const char *data, int len);
  */
 void util_version(void) __attribute__((noreturn));
 
+/**
+ * Show usage and exit
+ *
+ * This helps standardize the output of various utils.  You most likely want
+ * to use the long_usage() helper below rather than call this.
+ *
+ * @param errmsg	If non-NULL, an error message to display
+ * @param synopsis	The initial example usage text (and possible examples)
+ * @param short_opts	The string of short options
+ * @param long_opts	The structure of long options
+ * @param opts_help	An array of help strings (should align with long_opts)
+ */
+void util_long_usage(const char *errmsg, const char *synopsis,
+		     const char *short_opts, struct option const long_opts[],
+		     const char * const opts_help[]) __attribute__((noreturn));
+
+/**
+ * Show usage and exit
+ *
+ * If you name all your usage variables with usage_xxx, then you can call this
+ * help macro rather than expanding all arguments yourself.
+ *
+ * @param errmsg	If non-NULL, an error message to display
+ */
+#define long_usage(errmsg) \
+	util_long_usage(errmsg, usage_synopsis, usage_short_opts, \
+			usage_long_opts, usage_opts_help)
+
+/**
+ * Call getopt_long() with standard options
+ *
+ * Since all util code runs getopt in the same way, provide a helper.
+ */
+#define util_getopt_long() getopt_long(argc, argv, usage_short_opts, \
+				       usage_long_opts, NULL)
+
+/* Helper for aligning long_opts array */
+#define a_argument required_argument
+
+/* Helper for usage_short_opts string constant */
+#define USAGE_COMMON_SHORT_OPTS "hV"
+
+/* Helper for usage_long_opts option array */
+#define USAGE_COMMON_LONG_OPTS \
+	{"help",      no_argument, NULL, 'h'}, \
+	{"version",   no_argument, NULL, 'V'}, \
+	{NULL,        no_argument, NULL, 0x0}
+
+/* Helper for usage_opts_help array */
+#define USAGE_COMMON_OPTS_HELP \
+	"Print this help and exit", \
+	"Print version and exit", \
+	NULL
+
+/* Helper for getopt case statements */
+#define case_USAGE_COMMON_FLAGS \
+	case 'h': long_usage(NULL); \
+	case 'V': util_version(); \
+	case '?': long_usage("unknown option");
+
 #endif /* _UTIL_H */
-- 
1.8.1.2

Re: [PATCH 05/10] fdtdump: make usage a bit more friendly

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 789 bytes --]

On Mon, Apr 15, 2013 at 10:13:12PM -0400, Mike Frysinger wrote:
> This starts a new usage framework and then cuts fdtdump over to it.
> Now we can do `fdtdump -h` and get something useful back.
> 
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

So, I did some testing that convinced me these won't make it any
harder to fix the current bugs with the freebsd build (and might even
make it a little easier).  So:

Jon, please apply all the remainder of this series.

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 05/10] fdtdump: make usage a bit more friendly

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 965 bytes --]

On Tue, Apr 30, 2013 at 06:12:19PM +1000, David Gibson wrote:
> On Mon, Apr 15, 2013 at 10:13:12PM -0400, Mike Frysinger wrote:
> > This starts a new usage framework and then cuts fdtdump over to it.
> > Now we can do `fdtdump -h` and get something useful back.
> > 
> > Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> 
> Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> 
> So, I did some testing that convinced me these won't make it any
> harder to fix the current bugs with the freebsd build (and might even
> make it a little easier).  So:
> 
> Jon, please apply all the remainder of this series.

Jon?

This series and also at least one other patch is still waiting for
merge..

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 06/10] fdtdump: add a --scan option

[Mike Frysinger]

Often times, fdts get embedded in other larger files.  Rather than force
people to `dd` the blob out themselves, make the fdtdump file smarter.

It can now scan the blob looking for the fdt magic.  Once locate, it does
a little validation on the main struct to make sure we didn't hit random
binary data.

Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 fdtdump.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/fdtdump.c b/fdtdump.c
index a6e522c..c8a3ee7 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -2,12 +2,14 @@
  * fdtdump.c - Contributed by Pantelis Antoniou <pantelis.antoniou AT gmail.com>
  */
 
+#include <stdbool.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
 
+#include <libfdt.h>
 #include <libfdt_env.h>
 #include <fdt.h>
 
@@ -119,11 +121,13 @@ static void dump_blob(void *blob)
 
 /* Usage related data. */
 static const char usage_synopsis[] = "fdtdump [options] <file>";
-static const char usage_short_opts[] = USAGE_COMMON_SHORT_OPTS;
+static const char usage_short_opts[] = "s" USAGE_COMMON_SHORT_OPTS;
 static struct option const usage_long_opts[] = {
+	{"scan",             no_argument, NULL, 's'},
 	USAGE_COMMON_LONG_OPTS
 };
 static const char * const usage_opts_help[] = {
+	"Scan for an embedded fdt in file",
 	USAGE_COMMON_OPTS_HELP
 };
 
@@ -132,20 +136,58 @@ int main(int argc, char *argv[])
 	int opt;
 	const char *file;
 	char *buf;
+	bool scan = false;
+	off_t len;
 
 	while ((opt = util_getopt_long()) != EOF) {
 		switch (opt) {
 		case_USAGE_COMMON_FLAGS
+
+		case 's':
+			scan = true;
+			break;
 		}
 	}
 	if (optind != argc - 1)
 		long_usage("missing input filename");
 	file = argv[optind];
 
-	buf = utilfdt_read(file);
+	buf = utilfdt_read_len(file, &len);
 	if (!buf)
 		die("could not read: %s\n", file);
 
+	/* try and locate an embedded fdt in a bigger blob */
+	if (scan) {
+		unsigned char smagic[4];
+		char *p = buf;
+		char *endp = buf + len;
+
+		fdt_set_magic(smagic, FDT_MAGIC);
+
+		/* poor man's memmem */
+		while (true) {
+			p = memchr(p, smagic[0], endp - p - 4);
+			if (!p)
+				break;
+			if (fdt_magic(p) == FDT_MAGIC) {
+				/* try and validate the main struct */
+				off_t this_len = endp - p;
+				fdt32_t max_version = 17;
+				if (fdt_version(p) <= max_version &&
+				    fdt_last_comp_version(p) < max_version &&
+				    fdt_totalsize(p) < this_len &&
+				    fdt_off_dt_struct(p) < this_len &&
+					fdt_off_dt_strings(p) < this_len)
+					break;
+			}
+			++p;
+		}
+		if (!p)
+			die("%s: could not locate fdt magic\n", file);
+		printf("%s: found fdt at offset %#zx\n", file, p - buf);
+		buf = p;
+	}
+
 	dump_blob(buf);
 
 	return 0;
-- 
1.8.1.2

Re: [PATCH 06/10] fdtdump: add a --scan option

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 754 bytes --]

On Mon, Apr 15, 2013 at 10:13:13PM -0400, Mike Frysinger wrote:
> Often times, fdts get embedded in other larger files.  Rather than force
> people to `dd` the blob out themselves, make the fdtdump file smarter.
> 
> It can now scan the blob looking for the fdt magic.  Once locate, it does
> a little validation on the main struct to make sure we didn't hit random
> binary data.
> 
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 07/10] dtc/fdt{get, put}/convert-dtsv0-lexer: convert to new usage helpers

[Mike Frysinger]

This helps standardize the flag processing and the usage screens.

Only lightly tested; would be great if someone who uses these utils
could double check.

Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 convert-dtsv0-lexer.l |  22 +++++++---
 dtc.c                 | 111 ++++++++++++++++++++++++++------------------------
 fdtget.c              |  60 ++++++++++++---------------
 fdtput.c              |  63 +++++++++++++---------------
 util.h                |   2 +-
 5 files changed, 131 insertions(+), 127 deletions(-)

diff --git a/convert-dtsv0-lexer.l b/convert-dtsv0-lexer.l
index 89d540a..e62d27a 100644
--- a/convert-dtsv0-lexer.l
+++ b/convert-dtsv0-lexer.l
@@ -194,11 +194,15 @@ const struct {
 		}
 
 %%
-static void usage(void)
-{
-	fprintf(stderr, "convert-dtsv0 <v0 dts file>...\n");
-	exit(3);
-}
+/* Usage related data. */
+static const char usage_synopsis[] = "convert-dtsv0 [options] <v0 dts file>...";
+static const char usage_short_opts[] = "" USAGE_COMMON_SHORT_OPTS;
+static struct option const usage_long_opts[] = {
+	USAGE_COMMON_LONG_OPTS
+};
+static const char * const usage_opts_help[] = {
+	USAGE_COMMON_OPTS_HELP
+};
 
 static void convert_file(const char *fname)
 {
@@ -226,10 +230,16 @@ static void convert_file(const char *fname)
 
 int main(int argc, char *argv[])
 {
+	int opt;
 	int i;
 
+	while ((opt = util_getopt_long()) != EOF) {
+		switch (opt) {
+		case_USAGE_COMMON_FLAGS
+		}
+	}
 	if (argc < 2)
-		usage();
+		long_usage("missing filename");
 
 	for (i = 1; i < argc; i++) {
 		fprintf(stderr, "Converting %s from dts v0 to dts v1\n", argv[i]);
diff --git a/dtc.c b/dtc.c
index e4e1b84..d0a1f2d 100644
--- a/dtc.c
+++ b/dtc.c
@@ -47,55 +47,60 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
 		fill_fullpaths(child, tree->fullpath);
 }
 
-static void  __attribute__ ((noreturn)) usage(void)
-{
-	fprintf(stderr, "Usage:\n");
-	fprintf(stderr, "\tdtc [options] <input file>\n");
-	fprintf(stderr, "\nOptions:\n");
-	fprintf(stderr, "\t-h\n");
-	fprintf(stderr, "\t\tThis help text\n");
-	fprintf(stderr, "\t-q\n");
-	fprintf(stderr, "\t\tQuiet: -q suppress warnings, -qq errors, -qqq all\n");
-	fprintf(stderr, "\t-I <input format>\n");
-	fprintf(stderr, "\t\tInput formats are:\n");
-	fprintf(stderr, "\t\t\tdts - device tree source text\n");
-	fprintf(stderr, "\t\t\tdtb - device tree blob\n");
-	fprintf(stderr, "\t\t\tfs - /proc/device-tree style directory\n");
-	fprintf(stderr, "\t-o <output file>\n");
-	fprintf(stderr, "\t-O <output format>\n");
-	fprintf(stderr, "\t\tOutput formats are:\n");
-	fprintf(stderr, "\t\t\tdts - device tree source text\n");
-	fprintf(stderr, "\t\t\tdtb - device tree blob\n");
-	fprintf(stderr, "\t\t\tasm - assembler source\n");
-	fprintf(stderr, "\t-V <output version>\n");
-	fprintf(stderr, "\t\tBlob version to produce, defaults to %d (relevant for dtb\n\t\tand asm output only)\n", DEFAULT_FDT_VERSION);
-	fprintf(stderr, "\t-d <output dependency file>\n");
-	fprintf(stderr, "\t-R <number>\n");
-	fprintf(stderr, "\t\tMake space for <number> reserve map entries (relevant for \n\t\tdtb and asm output only)\n");
-	fprintf(stderr, "\t-S <bytes>\n");
-	fprintf(stderr, "\t\tMake the blob at least <bytes> long (extra space)\n");
-	fprintf(stderr, "\t-p <bytes>\n");
-	fprintf(stderr, "\t\tAdd padding to the blob of <bytes> long (extra space)\n");
-	fprintf(stderr, "\t-b <number>\n");
-	fprintf(stderr, "\t\tSet the physical boot cpu\n");
-	fprintf(stderr, "\t-f\n");
-	fprintf(stderr, "\t\tForce - try to produce output even if the input tree has errors\n");
-	fprintf(stderr, "\t-i\n");
-	fprintf(stderr, "\t\tAdd a path to search for include files\n");
-	fprintf(stderr, "\t-s\n");
-	fprintf(stderr, "\t\tSort nodes and properties before outputting (only useful for\n\t\tcomparing trees)\n");
-	fprintf(stderr, "\t-v\n");
-	fprintf(stderr, "\t\tPrint DTC version and exit\n");
-	fprintf(stderr, "\t-H <phandle format>\n");
-	fprintf(stderr, "\t\tphandle formats are:\n");
-	fprintf(stderr, "\t\t\tlegacy - \"linux,phandle\" properties only\n");
-	fprintf(stderr, "\t\t\tepapr - \"phandle\" properties only\n");
-	fprintf(stderr, "\t\t\tboth - Both \"linux,phandle\" and \"phandle\" properties\n");
-	fprintf(stderr, "\t-W [no-]<checkname>\n");
-	fprintf(stderr, "\t-E [no-]<checkname>\n");
-	fprintf(stderr, "\t\t\tenable or disable warnings and errors\n");
-	exit(3);
-}
+/* Usage related data. */
+static const char usage_synopsis[] = "dtc [options] <input file>";
+static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:hv";
+static struct option const usage_long_opts[] = {
+	{"quiet",            no_argument, NULL, 'q'},
+	{"in-format",         a_argument, NULL, 'I'},
+	{"out",               a_argument, NULL, 'o'},
+	{"out-format",        a_argument, NULL, 'O'},
+	{"out-version",       a_argument, NULL, 'V'},
+	{"out-dependency",    a_argument, NULL, 'd'},
+	{"reserve",           a_argument, NULL, 'R'},
+	{"space",             a_argument, NULL, 'S'},
+	{"pad",               a_argument, NULL, 'p'},
+	{"boot-cpu",          a_argument, NULL, 'b'},
+	{"force",            no_argument, NULL, 'f'},
+	{"include",           a_argument, NULL, 'i'},
+	{"sort",             no_argument, NULL, 's'},
+	{"phandle",           a_argument, NULL, 'H'},
+	{"warning",           a_argument, NULL, 'W'},
+	{"error",             a_argument, NULL, 'E'},
+	{"help",             no_argument, NULL, 'h'},
+	{"version",          no_argument, NULL, 'v'},
+	{NULL,               no_argument, NULL, 0x0},
+};
+static const char * const usage_opts_help[] = {
+	"\n\tQuiet: -q suppress warnings, -qq errors, -qqq all",
+	"\n\tInput formats are:\n"
+	 "\t\tdts - device tree source text\n"
+	 "\t\tdtb - device tree blob\n"
+	 "\t\tfs  - /proc/device-tree style directory",
+	"\n\tOutput file",
+	"\n\tOutput formats are:\n"
+	 "\t\tdts - device tree source text\n"
+	 "\t\tdtb - device tree blob\n"
+	 "\t\tasm - assembler source",
+	"\n\tBlob version to produce, defaults to %d (for dtb and asm output)", //, DEFAULT_FDT_VERSION);
+	"\n\tOutput dependency file",
+	"\n\ttMake space for <number> reserve map entries (for dtb and asm output)",
+	"\n\tMake the blob at least <bytes> long (extra space)",
+	"\n\tAdd padding to the blob of <bytes> long (extra space)",
+	"\n\tSet the physical boot cpu",
+	"\n\tTry to produce output even if the input tree has errors",
+	"\n\tAdd a path to search for include files",
+	"\n\tSort nodes and properties before outputting (useful for comparing trees)",
+	"\n\tValid phandle formats are:\n"
+	 "\t\tlegacy - \"linux,phandle\" properties only\n"
+	 "\t\tepapr  - \"phandle\" properties only\n"
+	 "\t\tboth   - Both \"linux,phandle\" and \"phandle\" properties",
+	"\n\tEnable/disable warnings (prefix with \"no-\")",
+	"\n\tEnable/disable errors (prefix with \"no-\")",
+	"\n\tPrint this help and exit",
+	"\n\tPrint version and exit",
+	NULL,
+};
 
 int main(int argc, char *argv[])
 {
@@ -116,8 +121,7 @@ int main(int argc, char *argv[])
 	minsize    = 0;
 	padsize    = 0;
 
-	while ((opt = getopt(argc, argv, "hI:O:o:V:d:R:S:p:fqb:i:vH:sW:E:"))
-			!= EOF) {
+	while ((opt = util_getopt_long()) != EOF) {
 		switch (opt) {
 		case 'I':
 			inform = optarg;
@@ -182,13 +186,14 @@ int main(int argc, char *argv[])
 			break;
 
 		case 'h':
+			long_usage(NULL);
 		default:
-			usage();
+			long_usage("unknown option");
 		}
 	}
 
 	if (argc > (optind+1))
-		usage();
+		long_usage("missing files");
 	else if (argc < (optind+1))
 		arg = "-";
 	else
diff --git a/fdtget.c b/fdtget.c
index c2fbab2..5008cc1 100644
--- a/fdtget.c
+++ b/fdtget.c
@@ -277,33 +277,33 @@ static int do_fdtget(struct display_info *disp, const char *filename,
 	return 0;
 }
 
-static const char *usage_msg =
-	"fdtget - read values from device tree\n"
-	"\n"
-	"Each value is printed on a new line.\n\n"
-	"Usage:\n"
+/* Usage related data. */
+static const char usage_synopsis[] =
+	"read values from device tree\n"
 	"	fdtget <options> <dt file> [<node> <property>]...\n"
 	"	fdtget -p <options> <dt file> [<node> ]...\n"
-	"Options:\n"
-	"\t-t <type>\tType of data\n"
-	"\t-p\t\tList properties for each node\n"
-	"\t-l\t\tList subnodes for each node\n"
-	"\t-d\t\tDefault value to display when the property is "
-			"missing\n"
-	"\t-h\t\tPrint this help\n\n"
+	"\n"
+	"Each value is printed on a new line.\n"
 	USAGE_TYPE_MSG;
-
-static void usage(const char *msg)
-{
-	if (msg)
-		fprintf(stderr, "Error: %s\n\n", msg);
-
-	fprintf(stderr, "%s", usage_msg);
-	exit(2);
-}
+static const char usage_short_opts[] = "t:pld:" USAGE_COMMON_SHORT_OPTS;
+static struct option const usage_long_opts[] = {
+	{"type",              a_argument, NULL, 't'},
+	{"properties",       no_argument, NULL, 'p'},
+	{"list",             no_argument, NULL, 'l'},
+	{"default",           a_argument, NULL, 'd'},
+	USAGE_COMMON_LONG_OPTS,
+};
+static const char * const usage_opts_help[] = {
+	"Type of data",
+	"List properties for each node",
+	"List subnodes for each node",
+	"Default value to display when the property is missing",
+	USAGE_COMMON_OPTS_HELP
+};
 
 int main(int argc, char *argv[])
 {
+	int opt;
 	char *filename = NULL;
 	struct display_info disp;
 	int args_per_step = 2;
@@ -312,20 +312,14 @@ int main(int argc, char *argv[])
 	memset(&disp, '\0', sizeof(disp));
 	disp.size = -1;
 	disp.mode = MODE_SHOW_VALUE;
-	for (;;) {
-		int c = getopt(argc, argv, "d:hlpt:");
-		if (c == -1)
-			break;
-
-		switch (c) {
-		case 'h':
-		case '?':
-			usage(NULL);
+	while ((opt = util_getopt_long()) != EOF) {
+		switch (opt) {
+		case_USAGE_COMMON_FLAGS
 
 		case 't':
 			if (utilfdt_decode_type(optarg, &disp.type,
 					&disp.size))
-				usage("Invalid type string");
+				long_usage("invalid type string");
 			break;
 
 		case 'p':
@@ -347,7 +341,7 @@ int main(int argc, char *argv[])
 	if (optind < argc)
 		filename = argv[optind++];
 	if (!filename)
-		usage("Missing filename");
+		long_usage("missing filename");
 
 	argv += optind;
 	argc -= optind;
@@ -358,7 +352,7 @@ int main(int argc, char *argv[])
 
 	/* Check for node, property arguments */
 	if (args_per_step == 2 && (argc % 2))
-		usage("Must have an even number of arguments");
+		long_usage("must have an even number of arguments");
 
 	if (do_fdtget(&disp, filename, argv, argc, args_per_step))
 		return 1;
diff --git a/fdtput.c b/fdtput.c
index f2197f5..f7bf56e 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -272,44 +272,40 @@ static int do_fdtput(struct display_info *disp, const char *filename,
 	return ret;
 }
 
-static const char *usage_msg =
-	"fdtput - write a property value to a device tree\n"
-	"\n"
-	"The command line arguments are joined together into a single value.\n"
-	"\n"
-	"Usage:\n"
+/* Usage related data. */
+static const char usage_synopsis[] =
+	"write a property value to a device tree\n"
 	"	fdtput <options> <dt file> <node> <property> [<value>...]\n"
 	"	fdtput -c <options> <dt file> [<node>...]\n"
-	"Options:\n"
-	"\t-c\t\tCreate nodes if they don't already exist\n"
-	"\t-p\t\tAutomatically create nodes as needed for the node path\n"
-	"\t-t <type>\tType of data\n"
-	"\t-v\t\tVerbose: display each value decoded from command line\n"
-	"\t-h\t\tPrint this help\n\n"
+	"\n"
+	"The command line arguments are joined together into a single value.\n"
 	USAGE_TYPE_MSG;
-
-static void usage(const char *msg)
-{
-	if (msg)
-		fprintf(stderr, "Error: %s\n\n", msg);
-
-	fprintf(stderr, "%s", usage_msg);
-	exit(2);
-}
+static const char usage_short_opts[] = "cpt:v" USAGE_COMMON_SHORT_OPTS;
+static struct option const usage_long_opts[] = {
+	{"create",           no_argument, NULL, 'c'},
+	{"auto-path",        no_argument, NULL, 'p'},
+	{"type",              a_argument, NULL, 't'},
+	{"verbose",          no_argument, NULL, 'v'},
+	USAGE_COMMON_LONG_OPTS,
+};
+static const char * const usage_opts_help[] = {
+	"Create nodes if they don't already exist",
+	"Automatically create nodes as needed for the node path",
+	"Type of data",
+	"Display each value decoded from command line",
+	USAGE_COMMON_OPTS_HELP
+};
 
 int main(int argc, char *argv[])
 {
+	int opt;
 	struct display_info disp;
 	char *filename = NULL;
 
 	memset(&disp, '\0', sizeof(disp));
 	disp.size = -1;
 	disp.oper = OPER_WRITE_PROP;
-	for (;;) {
-		int c = getopt(argc, argv, "chpt:v");
-		if (c == -1)
-			break;
-
+	while ((opt = util_getopt_long()) != EOF) {
 		/*
 		 * TODO: add options to:
 		 * - delete property
@@ -319,20 +315,19 @@ int main(int argc, char *argv[])
 		 * - set amount of free space when writing
 		 * - expand fdt if value doesn't fit
 		 */
-		switch (c) {
+		switch (opt) {
+		case_USAGE_COMMON_FLAGS
+
 		case 'c':
 			disp.oper = OPER_CREATE_NODE;
 			break;
-		case 'h':
-		case '?':
-			usage(NULL);
 		case 'p':
 			disp.auto_path = 1;
 			break;
 		case 't':
 			if (utilfdt_decode_type(optarg, &disp.type,
 					&disp.size))
-				usage("Invalid type string");
+				long_usage("invalid type string");
 			break;
 
 		case 'v':
@@ -344,16 +339,16 @@ int main(int argc, char *argv[])
 	if (optind < argc)
 		filename = argv[optind++];
 	if (!filename)
-		usage("Missing filename");
+		long_usage("missing filename");
 
 	argv += optind;
 	argc -= optind;
 
 	if (disp.oper == OPER_WRITE_PROP) {
 		if (argc < 1)
-			usage("Missing node");
+			long_usage("missing node");
 		if (argc < 2)
-			usage("Missing property");
+			long_usage("missing property");
 	}
 
 	if (do_fdtput(&disp, filename, argv, argc))
diff --git a/util.h b/util.h
index 1da3bd3..439b2b3 100644
--- a/util.h
+++ b/util.h
@@ -164,7 +164,7 @@ int utilfdt_decode_type(const char *fmt, int *type, int *size);
 #define USAGE_TYPE_MSG \
 	"<type>\ts=string, i=int, u=unsigned, x=hex\n" \
 	"\tOptional modifier prefix:\n" \
-	"\t\thh or b=byte, h=2 byte, l=4 byte (default)\n";
+	"\t\thh or b=byte, h=2 byte, l=4 byte (default)";
 
 /**
  * Print property data in a readable format to stdout
-- 
1.8.1.2

Re: [PATCH 07/10] dtc/fdt{get, put}/convert-dtsv0-lexer: convert to new usage helpers

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 598 bytes --]

On Mon, Apr 15, 2013 at 10:13:14PM -0400, Mike Frysinger wrote:
> This helps standardize the flag processing and the usage screens.
> 
> Only lightly tested; would be great if someone who uses these utils
> could double check.
> 
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 08/10] util: drop "long" from usage helpers

[Mike Frysinger]

Now that all utils have converted to the new usage framework, we can
rename to just plain "usage()" and avoid naming conflicts.

Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 convert-dtsv0-lexer.l |  2 +-
 dtc.c                 |  6 +++---
 fdtdump.c             |  2 +-
 fdtget.c              |  6 +++---
 fdtput.c              |  8 ++++----
 util.c                |  6 +++---
 util.h                | 18 +++++++++---------
 7 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/convert-dtsv0-lexer.l b/convert-dtsv0-lexer.l
index e62d27a..8902648 100644
--- a/convert-dtsv0-lexer.l
+++ b/convert-dtsv0-lexer.l
@@ -239,7 +239,7 @@ int main(int argc, char *argv[])
 		}
 	}
 	if (argc < 2)
-		long_usage("missing filename");
+		usage("missing filename");
 
 	for (i = 1; i < argc; i++) {
 		fprintf(stderr, "Converting %s from dts v0 to dts v1\n", argv[i]);
diff --git a/dtc.c b/dtc.c
index d0a1f2d..e3c9653 100644
--- a/dtc.c
+++ b/dtc.c
@@ -186,14 +186,14 @@ int main(int argc, char *argv[])
 			break;
 
 		case 'h':
-			long_usage(NULL);
+			usage(NULL);
 		default:
-			long_usage("unknown option");
+			usage("unknown option");
 		}
 	}
 
 	if (argc > (optind+1))
-		long_usage("missing files");
+		usage("missing files");
 	else if (argc < (optind+1))
 		arg = "-";
 	else
diff --git a/fdtdump.c b/fdtdump.c
index c8a3ee7..c2f16ea 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -149,7 +149,7 @@ int main(int argc, char *argv[])
 		}
 	}
 	if (optind != argc - 1)
-		long_usage("missing input filename");
+		usage("missing input filename");
 	file = argv[optind];
 
 	buf = utilfdt_read_len(file, &len);
diff --git a/fdtget.c b/fdtget.c
index 5008cc1..4377419 100644
--- a/fdtget.c
+++ b/fdtget.c
@@ -319,7 +319,7 @@ int main(int argc, char *argv[])
 		case 't':
 			if (utilfdt_decode_type(optarg, &disp.type,
 					&disp.size))
-				long_usage("invalid type string");
+				usage("invalid type string");
 			break;
 
 		case 'p':
@@ -341,7 +341,7 @@ int main(int argc, char *argv[])
 	if (optind < argc)
 		filename = argv[optind++];
 	if (!filename)
-		long_usage("missing filename");
+		usage("missing filename");
 
 	argv += optind;
 	argc -= optind;
@@ -352,7 +352,7 @@ int main(int argc, char *argv[])
 
 	/* Check for node, property arguments */
 	if (args_per_step == 2 && (argc % 2))
-		long_usage("must have an even number of arguments");
+		usage("must have an even number of arguments");
 
 	if (do_fdtget(&disp, filename, argv, argc, args_per_step))
 		return 1;
diff --git a/fdtput.c b/fdtput.c
index f7bf56e..ac1d98c 100644
--- a/fdtput.c
+++ b/fdtput.c
@@ -327,7 +327,7 @@ int main(int argc, char *argv[])
 		case 't':
 			if (utilfdt_decode_type(optarg, &disp.type,
 					&disp.size))
-				long_usage("invalid type string");
+				usage("invalid type string");
 			break;
 
 		case 'v':
@@ -339,16 +339,16 @@ int main(int argc, char *argv[])
 	if (optind < argc)
 		filename = argv[optind++];
 	if (!filename)
-		long_usage("missing filename");
+		usage("missing filename");
 
 	argv += optind;
 	argc -= optind;
 
 	if (disp.oper == OPER_WRITE_PROP) {
 		if (argc < 1)
-			long_usage("missing node");
+			usage("missing node");
 		if (argc < 2)
-			long_usage("missing property");
+			usage("missing property");
 	}
 
 	if (do_fdtput(&disp, filename, argv, argc))
diff --git a/util.c b/util.c
index d9a823a..3055c16 100644
--- a/util.c
+++ b/util.c
@@ -393,9 +393,9 @@ void util_version(void)
 	exit(0);
 }
 
-void util_long_usage(const char *errmsg, const char *synopsis,
-		     const char *short_opts, struct option const long_opts[],
-		     const char * const opts_help[])
+void util_usage(const char *errmsg, const char *synopsis,
+		const char *short_opts, struct option const long_opts[],
+		const char * const opts_help[])
 {
 	FILE *fp = errmsg ? stderr : stdout;
 	const char a_arg[] = "<arg>";
diff --git a/util.h b/util.h
index 439b2b3..7b9a840 100644
--- a/util.h
+++ b/util.h
@@ -189,7 +189,7 @@ void util_version(void) __attribute__((noreturn));
  * Show usage and exit
  *
  * This helps standardize the output of various utils.  You most likely want
- * to use the long_usage() helper below rather than call this.
+ * to use the usage() helper below rather than call this.
  *
  * @param errmsg	If non-NULL, an error message to display
  * @param synopsis	The initial example usage text (and possible examples)
@@ -197,9 +197,9 @@ void util_version(void) __attribute__((noreturn));
  * @param long_opts	The structure of long options
  * @param opts_help	An array of help strings (should align with long_opts)
  */
-void util_long_usage(const char *errmsg, const char *synopsis,
-		     const char *short_opts, struct option const long_opts[],
-		     const char * const opts_help[]) __attribute__((noreturn));
+void util_usage(const char *errmsg, const char *synopsis,
+		const char *short_opts, struct option const long_opts[],
+		const char * const opts_help[]) __attribute__((noreturn));
 
 /**
  * Show usage and exit
@@ -209,9 +209,9 @@ void util_long_usage(const char *errmsg, const char *synopsis,
  *
  * @param errmsg	If non-NULL, an error message to display
  */
-#define long_usage(errmsg) \
-	util_long_usage(errmsg, usage_synopsis, usage_short_opts, \
-			usage_long_opts, usage_opts_help)
+#define usage(errmsg) \
+	util_usage(errmsg, usage_synopsis, usage_short_opts, \
+		   usage_long_opts, usage_opts_help)
 
 /**
  * Call getopt_long() with standard options
@@ -241,8 +241,8 @@ void util_long_usage(const char *errmsg, const char *synopsis,
 
 /* Helper for getopt case statements */
 #define case_USAGE_COMMON_FLAGS \
-	case 'h': long_usage(NULL); \
+	case 'h': usage(NULL); \
 	case 'V': util_version(); \
-	case '?': long_usage("unknown option");
+	case '?': usage("unknown option");
 
 #endif /* _UTIL_H */
-- 
1.8.1.2

Re: [PATCH 08/10] util: drop "long" from usage helpers

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 565 bytes --]

On Mon, Apr 15, 2013 at 10:13:15PM -0400, Mike Frysinger wrote:
> Now that all utils have converted to the new usage framework, we can
> rename to just plain "usage()" and avoid naming conflicts.
> 
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 09/10] util: add common ARRAY_SIZE define

[Mike Frysinger]

I want to use this in more places, so put it in util.h rather than
copying & pasting it into another file.

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 convert-dtsv0-lexer.l | 2 --
 dtc.h                 | 1 -
 util.h                | 2 ++
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/convert-dtsv0-lexer.l b/convert-dtsv0-lexer.l
index 8902648..548e719 100644
--- a/convert-dtsv0-lexer.l
+++ b/convert-dtsv0-lexer.l
@@ -50,8 +50,6 @@ static int saw_hyphen; /* = 0 */
 static unsigned long long last_val;
 static char *last_name; /* = NULL */
 
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
-
 const struct {
 	const char *pattern;
 	int obase, width;
diff --git a/dtc.h b/dtc.h
index 3e42a07..264a20c 100644
--- a/dtc.h
+++ b/dtc.h
@@ -66,7 +66,6 @@ typedef uint32_t cell_t;
 #define strneq(a, b, n)	(strncmp((a), (b), (n)) == 0)
 
 #define ALIGN(x, a)	(((x) + (a) - 1) & ~((a) - 1))
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 /* Data blobs */
 enum markertype {
diff --git a/util.h b/util.h
index 7b9a840..8f40b44 100644
--- a/util.h
+++ b/util.h
@@ -24,6 +24,8 @@
  *                                                                   USA
  */
 
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+
 static inline void __attribute__((noreturn)) die(const char *str, ...)
 {
 	va_list ap;
-- 
1.8.1.2

Re: [PATCH 09/10] util: add common ARRAY_SIZE define

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 626 bytes --]

On Mon, Apr 15, 2013 at 10:13:16PM -0400, Mike Frysinger wrote:
> I want to use this in more places, so put it in util.h rather than
> copying & pasting it into another file.
> 
> Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>

Jon, this one too can go in right now, regardless of the rest of the
patches.

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 10/10] fdtdump: add a debug mode

[Mike Frysinger]

When hacking raw fdt files, it's useful to know the actual offsets into
the file each node appears.  Add a --debug mode that includes this.

Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 fdtdump.c | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/fdtdump.c b/fdtdump.c
index c2f16ea..723770d 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -19,8 +19,29 @@
 #define PALIGN(p, a)	((void *)(ALIGN((unsigned long)(p), (a))))
 #define GET_CELL(p)	(p += 4, *((const uint32_t *)(p-4)))
 
-static void dump_blob(void *blob)
+static const char *tagname(uint32_t tag)
 {
+	static const char * const names[] = {
+#define TN(t) [t] #t
+		TN(FDT_BEGIN_NODE),
+		TN(FDT_END_NODE),
+		TN(FDT_PROP),
+		TN(FDT_NOP),
+		TN(FDT_END),
+#undef TN
+	};
+	if (tag < ARRAY_SIZE(names))
+		if (names[tag])
+			return names[tag];
+	return "FDT_???";
+}
+
+#define dumpf(fmt, args...) \
+	do { if (debug) printf("// " fmt, ## args); } while (0)
+
+static void dump_blob(void *blob, bool debug)
+{
+	uintptr_t blob_off = (uintptr_t)blob;
 	struct fdt_header *bph = blob;
 	uint32_t off_mem_rsvmap = fdt32_to_cpu(bph->off_mem_rsvmap);
 	uint32_t off_dt = fdt32_to_cpu(bph->off_dt_struct);
@@ -74,7 +95,8 @@ static void dump_blob(void *blob)
 	p = p_struct;
 	while ((tag = fdt32_to_cpu(GET_CELL(p))) != FDT_END) {
 
-		/* printf("tag: 0x%08x (%d)\n", tag, p - p_struct); */
+		dumpf("%04zx: tag: 0x%08x (%s)\n",
+		        (uintptr_t)p - blob_off - 4, tag, tagname(tag));
 
 		if (tag == FDT_BEGIN_NODE) {
 			s = p;
@@ -113,6 +135,8 @@ static void dump_blob(void *blob)
 
 		p = PALIGN(p + sz, 4);
 
+		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);
 		utilfdt_print_data(t, sz);
 		printf(";\n");
@@ -121,12 +145,14 @@ static void dump_blob(void *blob)
 
 /* Usage related data. */
 static const char usage_synopsis[] = "fdtdump [options] <file>";
-static const char usage_short_opts[] = "s" USAGE_COMMON_SHORT_OPTS;
+static const char usage_short_opts[] = "ds" USAGE_COMMON_SHORT_OPTS;
 static struct option const usage_long_opts[] = {
+	{"debug",            no_argument, NULL, 'd'},
 	{"scan",             no_argument, NULL, 's'},
 	USAGE_COMMON_LONG_OPTS
 };
 static const char * const usage_opts_help[] = {
+	"Dump debug information while decoding the file",
 	"Scan for an embedded fdt in file",
 	USAGE_COMMON_OPTS_HELP
 };
@@ -136,6 +162,7 @@ int main(int argc, char *argv[])
 	int opt;
 	const char *file;
 	char *buf;
+	bool debug = false;
 	bool scan = false;
 	off_t len;
 
@@ -143,6 +170,9 @@ int main(int argc, char *argv[])
 		switch (opt) {
 		case_USAGE_COMMON_FLAGS
 
+		case 'd':
+			debug = true;
+			break;
 		case 's':
 			scan = true;
 			break;
@@ -179,6 +209,9 @@ int main(int argc, char *argv[])
 				    fdt_off_dt_struct(p) < this_len &&
 					fdt_off_dt_strings(p) < this_len)
 					break;
+				if (debug)
+					printf("%s: skipping fdt magic at offset %#zx\n",
+						file, p - buf);
 			}
 			++p;
 		}
@@ -188,7 +221,7 @@ int main(int argc, char *argv[])
 		buf = p;
 	}
 
-	dump_blob(buf);
+	dump_blob(buf, debug);
 
 	return 0;
 }
-- 
1.8.1.2

Re: [PATCH 10/10] fdtdump: add a debug mode

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 577 bytes --]

On Mon, Apr 15, 2013 at 10:13:17PM -0400, Mike Frysinger wrote:
> When hacking raw fdt files, it's useful to know the actual offsets into
> the file each node appears.  Add a --debug mode that includes this.
> 
> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>

Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 00/10 v2] usage()/--help clean up & unification

[Mike Frysinger]

Each utility currently open codes how they process options and
implement usage().  This leads to inconsistent behavior and output
which sucks.  A bunch also get common things wrong (like sending
--help to stderr).

Rather than go through them one by one and fix their bugs, start
a new mini framework in the util.[ch] code base.  This allows the
utils to be much more data driven in their approach to options --
they declare certain structures with the right format, and then
call usage() as needed.  The new util code takes care of the rest.

I also snuck in a few minor fixes in this patch series that I
noticed as I was hacking on things.  Really I just wanted a --scan
option in the fdtdump tool :).

For the v2 series, I fixed a compile/output bug in one of the
patches and this now passes `make check`.  I also added a --debug
option to fdtdump.

Mike Frysinger (10):
  utilfdt_read_err: use xmalloc funcs
  utilfdt_read: pass back up the length of data read
  die: constify format string arg
  util_version: new helper for displaying version info
  fdtdump: make usage a bit more friendly
  fdtdump: add a --scan option
  dtc/fdt{get,put}/convert-dtsv0-lexer: convert to new usage helpers
  util: drop "long" from usage helpers
  util: add common ARRAY_SIZE define
  fdtdump: add a debug mode

 convert-dtsv0-lexer.l |  24 +++++++----
 dtc.c                 | 116 +++++++++++++++++++++++++-------------------------
 dtc.h                 |   1 -
 fdtdump.c             | 109 ++++++++++++++++++++++++++++++++++++++++++-----
 fdtget.c              |  62 ++++++++++++---------------
 fdtput.c              |  65 +++++++++++++---------------
 tests/testutils.c     |   2 +-
 util.c                |  73 ++++++++++++++++++++++++++++---
 util.h                |  78 +++++++++++++++++++++++++++++++--
 9 files changed, 375 insertions(+), 155 deletions(-)

-- 
1.8.1.2

[PATCH 10/10] fdtdump: add a debug mode

[Mike Frysinger]

When hacking raw fdt files, it's useful to know the actual offsets into
the file each node appears.  Add a --debug mode that includes this.

Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
---
 fdtdump.c | 40 ++++++++++++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 4 deletions(-)

diff --git a/fdtdump.c b/fdtdump.c
index 2b8c194..2c8bff6 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -19,8 +19,29 @@
 #define PALIGN(p, a)	((void *)(ALIGN((unsigned long)(p), (a))))
 #define GET_CELL(p)	(p += 4, *((const uint32_t *)(p-4)))
 
-static void dump_blob(void *blob)
+static const char *tagname(uint32_t tag)
 {
+	static const char * const names[] = {
+#define TN(t) [t] #t
+		TN(FDT_BEGIN_NODE),
+		TN(FDT_END_NODE),
+		TN(FDT_PROP),
+		TN(FDT_NOP),
+		TN(FDT_END),
+#undef TN
+	};
+	if (tag < ARRAY_SIZE(names))
+		if (names[tag])
+			return names[tag];
+	return "???";
+}
+
+#define dprintf(fmt, args...) \
+	do { if (debug) printf("// " fmt, ## args); } while (0)
+
+static void dump_blob(void *blob, bool debug)
+{
+	uintptr_t blob_off = (uintptr_t)blob;
 	struct fdt_header *bph = blob;
 	uint32_t off_mem_rsvmap = fdt32_to_cpu(bph->off_mem_rsvmap);
 	uint32_t off_dt = fdt32_to_cpu(bph->off_dt_struct);
@@ -74,7 +95,8 @@ static void dump_blob(void *blob)
 	p = p_struct;
 	while ((tag = fdt32_to_cpu(GET_CELL(p))) != FDT_END) {
 
-		/* printf("tag: 0x%08x (%d)\n", tag, p - p_struct); */
+		dprintf("%04zx: tag: 0x%08x (%s)\n",
+		        (uintptr_t)p - blob_off - 4, tag, tagname(tag));
 
 		if (tag == FDT_BEGIN_NODE) {
 			s = p;
@@ -113,6 +135,8 @@ static void dump_blob(void *blob)
 
 		p = PALIGN(p + sz, 4);
 
+		dprintf("%04zx: string: %s\n", (uintptr_t)s - blob_off, s);
+		dprintf("%04zx: value\n", (uintptr_t)t - blob_off);
 		printf("%*s%s", depth * shift, "", s);
 		utilfdt_print_data(t, sz);
 		printf(";\n");
@@ -121,8 +145,9 @@ static void dump_blob(void *blob)
 
 /* Usage related data. */
 static const char usage_synopsis[] = "fdtdump [options] <file>";
-static const char usage_short_opts[] = "s" USAGE_COMMON_SHORT_OPTS;
+static const char usage_short_opts[] = "ds" USAGE_COMMON_SHORT_OPTS;
 static struct option const usage_long_opts[] = {
+	{"debug",            no_argument, NULL, 'd'},
 	{"scan",             no_argument, NULL, 's'},
 	USAGE_COMMON_LONG_OPTS
 };
@@ -136,6 +161,7 @@ int main(int argc, char *argv[])
 	int opt;
 	const char *file;
 	char *buf;
+	bool debug = false;
 	bool scan = false;
 	off_t len;
 
@@ -143,6 +169,9 @@ int main(int argc, char *argv[])
 		switch (opt) {
 		case_USAGE_COMMON_FLAGS
 
+		case 'd':
+			debug = true;
+			break;
 		case 's':
 			scan = true;
 			break;
@@ -179,6 +208,9 @@ int main(int argc, char *argv[])
 				    fdt_off_dt_struct(p) < this_len &&
 					fdt_off_dt_strings(p) < this_len)
 					break;
+				if (debug)
+					printf("%s: skipping fdt magic at offset %#zx\n",
+						file, p - buf);
 			}
 			++p;
 		}
@@ -188,7 +220,7 @@ int main(int argc, char *argv[])
 		buf = p;
 	}
 
-	dump_blob(buf);
+	dump_blob(buf, debug);
 
 	return 0;
 }
-- 
1.8.1.2

Re: [PATCH 10/10] fdtdump: add a debug mode

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 4236 bytes --]

On Wed, Apr 10, 2013 at 02:29:15PM -0400, Mike Frysinger wrote:
> When hacking raw fdt files, it's useful to know the actual offsets into
> the file each node appears.  Add a --debug mode that includes this.

Neat idea.  Few comments on implementation below.

> Signed-off-by: Mike Frysinger <vapier-aBrp7R+bbdUdnm+yROfE0A@public.gmane.org>
> ---
>  fdtdump.c | 40 ++++++++++++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/fdtdump.c b/fdtdump.c
> index 2b8c194..2c8bff6 100644
> --- a/fdtdump.c
> +++ b/fdtdump.c
> @@ -19,8 +19,29 @@
>  #define PALIGN(p, a)	((void *)(ALIGN((unsigned long)(p), (a))))
>  #define GET_CELL(p)	(p += 4, *((const uint32_t *)(p-4)))
>  
> -static void dump_blob(void *blob)
> +static const char *tagname(uint32_t tag)
>  {
> +	static const char * const names[] = {
> +#define TN(t) [t] #t
> +		TN(FDT_BEGIN_NODE),
> +		TN(FDT_END_NODE),
> +		TN(FDT_PROP),
> +		TN(FDT_NOP),
> +		TN(FDT_END),
> +#undef TN
> +	};
> +	if (tag < ARRAY_SIZE(names))
> +		if (names[tag])
> +			return names[tag];
> +	return "???";

Better to return NULL here, I think.  That way a caller can easily
check and print the numeric value instead.

Or you could construct a string with the number.  It would leak
memory, obviously, but in a short-run program like this, it doesn't
really matter.

> +}
> +
> +#define dprintf(fmt, args...) \
> +	do { if (debug) printf("// " fmt, ## args); } while (0)

I'd prefer a different function (well, macro) name.  I tend to use
"dprintf" for debug print functions in the sense of debugging for the
program they're in, whereas in this case it's for the debug mode which
is (primarily) for debugging other programs.

> +static void dump_blob(void *blob, bool debug)
> +{
> +	uintptr_t blob_off = (uintptr_t)blob;
>  	struct fdt_header *bph = blob;
>  	uint32_t off_mem_rsvmap = fdt32_to_cpu(bph->off_mem_rsvmap);
>  	uint32_t off_dt = fdt32_to_cpu(bph->off_dt_struct);
> @@ -74,7 +95,8 @@ static void dump_blob(void *blob)
>  	p = p_struct;
>  	while ((tag = fdt32_to_cpu(GET_CELL(p))) != FDT_END) {
>  
> -		/* printf("tag: 0x%08x (%d)\n", tag, p - p_struct); */
> +		dprintf("%04zx: tag: 0x%08x (%s)\n",
> +		        (uintptr_t)p - blob_off - 4, tag, tagname(tag));
>  
>  		if (tag == FDT_BEGIN_NODE) {
>  			s = p;
> @@ -113,6 +135,8 @@ static void dump_blob(void *blob)
>  
>  		p = PALIGN(p + sz, 4);
>  
> +		dprintf("%04zx: string: %s\n", (uintptr_t)s - blob_off, s);
> +		dprintf("%04zx: value\n", (uintptr_t)t - blob_off);
>  		printf("%*s%s", depth * shift, "", s);
>  		utilfdt_print_data(t, sz);
>  		printf(";\n");
> @@ -121,8 +145,9 @@ static void dump_blob(void *blob)
>  
>  /* Usage related data. */
>  static const char usage_synopsis[] = "fdtdump [options] <file>";
> -static const char usage_short_opts[] = "s" USAGE_COMMON_SHORT_OPTS;
> +static const char usage_short_opts[] = "ds" USAGE_COMMON_SHORT_OPTS;
>  static struct option const usage_long_opts[] = {
> +	{"debug",            no_argument, NULL, 'd'},
>  	{"scan",             no_argument, NULL, 's'},
>  	USAGE_COMMON_LONG_OPTS
>  };
> @@ -136,6 +161,7 @@ int main(int argc, char *argv[])
>  	int opt;
>  	const char *file;
>  	char *buf;
> +	bool debug = false;
>  	bool scan = false;
>  	off_t len;
>  
> @@ -143,6 +169,9 @@ int main(int argc, char *argv[])
>  		switch (opt) {
>  		case_USAGE_COMMON_FLAGS
>  
> +		case 'd':
> +			debug = true;
> +			break;
>  		case 's':
>  			scan = true;
>  			break;
> @@ -179,6 +208,9 @@ int main(int argc, char *argv[])
>  				    fdt_off_dt_struct(p) < this_len &&
>  					fdt_off_dt_strings(p) < this_len)
>  					break;
> +				if (debug)
> +					printf("%s: skipping fdt magic at offset %#zx\n",
> +						file, p - buf);
>  			}
>  			++p;
>  		}
> @@ -188,7 +220,7 @@ int main(int argc, char *argv[])
>  		buf = p;
>  	}
>  
> -	dump_blob(buf);
> +	dump_blob(buf, debug);
>  
>  	return 0;
>  }

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 10/10] fdtdump: add a debug mode

[Mike Frysinger]

[-- Attachment #1.1: Type: Text/Plain, Size: 1621 bytes --]

On Monday 15 April 2013 01:12:06 David Gibson wrote:
> On Wed, Apr 10, 2013 at 02:29:15PM -0400, Mike Frysinger wrote:
> > -static void dump_blob(void *blob)
> > +static const char *tagname(uint32_t tag)
> >  {
> > +	static const char * const names[] = {
> > +#define TN(t) [t] #t
> > +		TN(FDT_BEGIN_NODE),
> > +		TN(FDT_END_NODE),
> > +		TN(FDT_PROP),
> > +		TN(FDT_NOP),
> > +		TN(FDT_END),
> > +#undef TN
> > +	};
> > +	if (tag < ARRAY_SIZE(names))
> > +		if (names[tag])
> > +			return names[tag];
> > +	return "???";
> 
> Better to return NULL here, I think.  That way a caller can easily
> check and print the numeric value instead.
> 
> Or you could construct a string with the number.  It would leak
> memory, obviously, but in a short-run program like this, it doesn't
> really matter.

if this were a general func (like in util.c or something), i'd agree with you.  
but the only consumer also prints out the #, so it'd be redundant:
        dprintf("%04zx: tag: 0x%08x (%s)\n",
                (uintptr_t)p - blob_off - 4, tag, tagname(tag));

the output looks like:
	// 350c: tag: 0x00000003 (FDT_PROP)

if it was an unknown value, it'd look like:
	// 350c: tag: 0x0000a123 (???)

> > +#define dprintf(fmt, args...) \
> > +	do { if (debug) printf("// " fmt, ## args); } while (0)
> 
> I'd prefer a different function (well, macro) name.  I tend to use
> "dprintf" for debug print functions in the sense of debugging for the
> program they're in, whereas in this case it's for the debug mode which
> is (primarily) for debugging other programs.

dumpf() ?
-mike

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 10/10] fdtdump: add a debug mode

[David Gibson]

[-- Attachment #1.1: Type: text/plain, Size: 2068 bytes --]

On Mon, Apr 15, 2013 at 07:35:55PM -0400, Mike Frysinger wrote:
> On Monday 15 April 2013 01:12:06 David Gibson wrote:
> > On Wed, Apr 10, 2013 at 02:29:15PM -0400, Mike Frysinger wrote:
> > > -static void dump_blob(void *blob)
> > > +static const char *tagname(uint32_t tag)
> > >  {
> > > +	static const char * const names[] = {
> > > +#define TN(t) [t] #t
> > > +		TN(FDT_BEGIN_NODE),
> > > +		TN(FDT_END_NODE),
> > > +		TN(FDT_PROP),
> > > +		TN(FDT_NOP),
> > > +		TN(FDT_END),
> > > +#undef TN
> > > +	};
> > > +	if (tag < ARRAY_SIZE(names))
> > > +		if (names[tag])
> > > +			return names[tag];
> > > +	return "???";
> > 
> > Better to return NULL here, I think.  That way a caller can easily
> > check and print the numeric value instead.
> > 
> > Or you could construct a string with the number.  It would leak
> > memory, obviously, but in a short-run program like this, it doesn't
> > really matter.
> 
> if this were a general func (like in util.c or something), i'd agree with you.  
> but the only consumer also prints out the #, so it'd be redundant:
>         dprintf("%04zx: tag: 0x%08x (%s)\n",
>                 (uintptr_t)p - blob_off - 4, tag, tagname(tag));
> 
> the output looks like:
> 	// 350c: tag: 0x00000003 (FDT_PROP)
> 
> if it was an unknown value, it'd look like:
> 	// 350c: tag: 0x0000a123 (???)

Ah, good point, fair enough.

> > > +#define dprintf(fmt, args...) \
> > > +	do { if (debug) printf("// " fmt, ## args); } while (0)
> > 
> > I'd prefer a different function (well, macro) name.  I tend to use
> > "dprintf" for debug print functions in the sense of debugging for the
> > program they're in, whereas in this case it's for the debug mode which
> > is (primarily) for debugging other programs.
> 
> dumpf() ?

Not great, but nothing better springs to mind, so yeah, that'll do.

-- 
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 #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 192 bytes --]

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss