[PATCH] libfdt: introduce fdt type annotation for use by endian checkers

[PATCH] libfdt: introduce fdt type annotation for use by endian checkers

[Kim Phillips]

Projects such as linux and u-boot run sparse on libfdt.  libfdt
contains the notion of endianness via usage of endian conversion
functions such as fdt32_to_cpu.  As such, in order to pass endian
checks, libfdt has to annotate its fdt variables as big endian.
This patch does that ifdef __CHECKER__ (a symbol sparse defines),
for two new fdt types: fdt32_t and fdt64_t, and subsequently
silences warnings emitted by sparse when parsing libfdt.

Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
---
note: wasn't sure whether to introduce the new fdt32 types, or
just have libfdt use __be32 directly.

 libfdt/fdt.c     |  2 +-
 libfdt/fdt.h     | 50 +++++++++++++++++++++++++++++---------------------
 libfdt/fdt_ro.c  |  2 +-
 libfdt/fdt_rw.c  |  4 ++--
 libfdt/fdt_sw.c  |  4 ++--
 libfdt/fdt_wip.c |  2 +-
 libfdt/libfdt.h  | 32 ++++++++++++++++----------------
 7 files changed, 52 insertions(+), 44 deletions(-)

diff --git a/libfdt/fdt.c b/libfdt/fdt.c
index e56833a..57faba3 100644
--- a/libfdt/fdt.c
+++ b/libfdt/fdt.c
@@ -92,7 +92,7 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
 
 uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
 {
-	const uint32_t *tagp, *lenp;
+	const fdt32_t *tagp, *lenp;
 	uint32_t tag;
 	int offset = startoffset;
 	const char *p;
diff --git a/libfdt/fdt.h b/libfdt/fdt.h
index 48ccfd9..0d9c856 100644
--- a/libfdt/fdt.h
+++ b/libfdt/fdt.h
@@ -3,46 +3,54 @@
 
 #ifndef __ASSEMBLY__
 
+#ifdef __CHECKER__
+typedef __be32 fdt32_t;
+typedef __be64 fdt64_t;
+#else
+typedef uint32_t fdt32_t;
+typedef uint64_t fdt64_t;
+#endif
+
 struct fdt_header {
-	uint32_t magic;			 /* magic word FDT_MAGIC */
-	uint32_t totalsize;		 /* total size of DT block */
-	uint32_t off_dt_struct;		 /* offset to structure */
-	uint32_t off_dt_strings;	 /* offset to strings */
-	uint32_t off_mem_rsvmap;	 /* offset to memory reserve map */
-	uint32_t version;		 /* format version */
-	uint32_t last_comp_version;	 /* last compatible version */
+	fdt32_t magic;			 /* magic word FDT_MAGIC */
+	fdt32_t totalsize;		 /* total size of DT block */
+	fdt32_t off_dt_struct;		 /* offset to structure */
+	fdt32_t off_dt_strings;		 /* offset to strings */
+	fdt32_t off_mem_rsvmap;		 /* offset to memory reserve map */
+	fdt32_t version;		 /* format version */
+	fdt32_t last_comp_version;	 /* last compatible version */
 
 	/* version 2 fields below */
-	uint32_t boot_cpuid_phys;	 /* Which physical CPU id we're
+	fdt32_t boot_cpuid_phys;	 /* Which physical CPU id we're
 					    booting on */
 	/* version 3 fields below */
-	uint32_t size_dt_strings;	 /* size of the strings block */
+	fdt32_t size_dt_strings;	 /* size of the strings block */
 
 	/* version 17 fields below */
-	uint32_t size_dt_struct;	 /* size of the structure block */
+	fdt32_t size_dt_struct;		 /* size of the structure block */
 };
 
 struct fdt_reserve_entry {
-	uint64_t address;
-	uint64_t size;
+	fdt64_t address;
+	fdt64_t size;
 };
 
 struct fdt_node_header {
-	uint32_t tag;
+	fdt32_t tag;
 	char name[0];
 };
 
 struct fdt_property {
-	uint32_t tag;
-	uint32_t len;
-	uint32_t nameoff;
+	fdt32_t tag;
+	fdt32_t len;
+	fdt32_t nameoff;
 	char data[0];
 };
 
 #endif /* !__ASSEMBLY */
 
 #define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
-#define FDT_TAGSIZE	sizeof(uint32_t)
+#define FDT_TAGSIZE	sizeof(fdt32_t)
 
 #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
 #define FDT_END_NODE	0x2		/* End node */
@@ -51,10 +59,10 @@ struct fdt_property {
 #define FDT_NOP		0x4		/* nop */
 #define FDT_END		0x9
 
-#define FDT_V1_SIZE	(7*sizeof(uint32_t))
-#define FDT_V2_SIZE	(FDT_V1_SIZE + sizeof(uint32_t))
-#define FDT_V3_SIZE	(FDT_V2_SIZE + sizeof(uint32_t))
+#define FDT_V1_SIZE	(7*sizeof(fdt32_t))
+#define FDT_V2_SIZE	(FDT_V1_SIZE + sizeof(fdt32_t))
+#define FDT_V3_SIZE	(FDT_V2_SIZE + sizeof(fdt32_t))
 #define FDT_V16_SIZE	FDT_V3_SIZE
-#define FDT_V17_SIZE	(FDT_V16_SIZE + sizeof(uint32_t))
+#define FDT_V17_SIZE	(FDT_V16_SIZE + sizeof(fdt32_t))
 
 #endif /* _FDT_H */
diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index 02b6d68..42da2bd 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -322,7 +322,7 @@ const void *fdt_getprop(const void *fdt, int nodeoffset,
 
 uint32_t fdt_get_phandle(const void *fdt, int nodeoffset)
 {
-	const uint32_t *php;
+	const fdt32_t *php;
 	int len;
 
 	/* FIXME: This is a bit sub-optimal, since we potentially scan
diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index 24437df..fdba618 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -339,7 +339,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
 	int nodelen;
 	int err;
 	uint32_t tag;
-	uint32_t *endtag;
+	fdt32_t *endtag;
 
 	FDT_RW_CHECK_HEADER(fdt);
 
@@ -366,7 +366,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
 	nh->tag = cpu_to_fdt32(FDT_BEGIN_NODE);
 	memset(nh->name, 0, FDT_TAGALIGN(namelen+1));
 	memcpy(nh->name, name, namelen);
-	endtag = (uint32_t *)((char *)nh + nodelen - FDT_TAGSIZE);
+	endtag = (fdt32_t *)((char *)nh + nodelen - FDT_TAGSIZE);
 	*endtag = cpu_to_fdt32(FDT_END_NODE);
 
 	return offset;
diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c
index 55ebebf..f422754 100644
--- a/libfdt/fdt_sw.c
+++ b/libfdt/fdt_sw.c
@@ -153,7 +153,7 @@ int fdt_begin_node(void *fdt, const char *name)
 
 int fdt_end_node(void *fdt)
 {
-	uint32_t *en;
+	fdt32_t *en;
 
 	FDT_SW_CHECK_HEADER(fdt);
 
@@ -213,7 +213,7 @@ int fdt_property(void *fdt, const char *name, const void *val, int len)
 int fdt_finish(void *fdt)
 {
 	char *p = (char *)fdt;
-	uint32_t *end;
+	fdt32_t *end;
 	int oldstroffset, newstroffset;
 	uint32_t tag;
 	int offset, nextoffset;
diff --git a/libfdt/fdt_wip.c b/libfdt/fdt_wip.c
index 6025fa1..c5bbb68 100644
--- a/libfdt/fdt_wip.c
+++ b/libfdt/fdt_wip.c
@@ -74,7 +74,7 @@ int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
 
 static void _fdt_nop_region(void *start, int len)
 {
-	uint32_t *p;
+	fdt32_t *p;
 
 	for (p = start; (char *)p < ((char *)start + len); p++)
 		*p = cpu_to_fdt32(FDT_NOP);
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 73f4975..8e57a06 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -882,8 +882,8 @@ int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
 static inline int fdt_setprop_inplace_u32(void *fdt, int nodeoffset,
 					  const char *name, uint32_t val)
 {
-	val = cpu_to_fdt32(val);
-	return fdt_setprop_inplace(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt32_t tmp = cpu_to_fdt32(val);
+	return fdt_setprop_inplace(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
@@ -917,8 +917,8 @@ static inline int fdt_setprop_inplace_u32(void *fdt, int nodeoffset,
 static inline int fdt_setprop_inplace_u64(void *fdt, int nodeoffset,
 					  const char *name, uint64_t val)
 {
-	val = cpu_to_fdt64(val);
-	return fdt_setprop_inplace(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt64_t tmp = cpu_to_fdt64(val);
+	return fdt_setprop_inplace(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
@@ -993,13 +993,13 @@ int fdt_begin_node(void *fdt, const char *name);
 int fdt_property(void *fdt, const char *name, const void *val, int len);
 static inline int fdt_property_u32(void *fdt, const char *name, uint32_t val)
 {
-	val = cpu_to_fdt32(val);
-	return fdt_property(fdt, name, &val, sizeof(val));
+	fdt32_t tmp = cpu_to_fdt32(val);
+	return fdt_property(fdt, name, &tmp, sizeof(tmp));
 }
 static inline int fdt_property_u64(void *fdt, const char *name, uint64_t val)
 {
-	val = cpu_to_fdt64(val);
-	return fdt_property(fdt, name, &val, sizeof(val));
+	fdt64_t tmp = cpu_to_fdt64(val);
+	return fdt_property(fdt, name, &tmp, sizeof(tmp));
 }
 static inline int fdt_property_cell(void *fdt, const char *name, uint32_t val)
 {
@@ -1154,8 +1154,8 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
 static inline int fdt_setprop_u32(void *fdt, int nodeoffset, const char *name,
 				  uint32_t val)
 {
-	val = cpu_to_fdt32(val);
-	return fdt_setprop(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt32_t tmp = cpu_to_fdt32(val);
+	return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
@@ -1189,8 +1189,8 @@ static inline int fdt_setprop_u32(void *fdt, int nodeoffset, const char *name,
 static inline int fdt_setprop_u64(void *fdt, int nodeoffset, const char *name,
 				  uint64_t val)
 {
-	val = cpu_to_fdt64(val);
-	return fdt_setprop(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt64_t tmp = cpu_to_fdt64(val);
+	return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
@@ -1296,8 +1296,8 @@ int fdt_appendprop(void *fdt, int nodeoffset, const char *name,
 static inline int fdt_appendprop_u32(void *fdt, int nodeoffset,
 				     const char *name, uint32_t val)
 {
-	val = cpu_to_fdt32(val);
-	return fdt_appendprop(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt32_t tmp = cpu_to_fdt32(val);
+	return fdt_appendprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
@@ -1331,8 +1331,8 @@ static inline int fdt_appendprop_u32(void *fdt, int nodeoffset,
 static inline int fdt_appendprop_u64(void *fdt, int nodeoffset,
 				     const char *name, uint64_t val)
 {
-	val = cpu_to_fdt64(val);
-	return fdt_appendprop(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt64_t tmp = cpu_to_fdt64(val);
+	return fdt_appendprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
-- 
1.8.0

Re: [PATCH] libfdt: introduce fdt type annotation for use by endian checkers

[Stephen Warren]

On 10/30/2012 03:57 PM, Kim Phillips wrote:
> Projects such as linux and u-boot run sparse on libfdt.  libfdt
> contains the notion of endianness via usage of endian conversion
> functions such as fdt32_to_cpu.  As such, in order to pass endian
> checks, libfdt has to annotate its fdt variables as big endian.
> This patch does that ifdef __CHECKER__ (a symbol sparse defines),
> for two new fdt types: fdt32_t and fdt64_t, and subsequently
> silences warnings emitted by sparse when parsing libfdt.

Should libfdt patches be committed to the main dtc repository (which I
assume is also upstream for libfdt?) rather than U-Boot first?
Otherwise, if we want to bring in a new libfdt from upstream, that would
trash all the U-Boot-specific changes in U-Boot's copy of libfdt.

Re: [PATCH] libfdt: introduce fdt type annotation for use by endian checkers

[Kim Phillips]

On Tue, 30 Oct 2012 16:24:05 -0600
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 10/30/2012 03:57 PM, Kim Phillips wrote:
> > Projects such as linux and u-boot run sparse on libfdt.  libfdt
> > contains the notion of endianness via usage of endian conversion
> > functions such as fdt32_to_cpu.  As such, in order to pass endian
> > checks, libfdt has to annotate its fdt variables as big endian.
> > This patch does that ifdef __CHECKER__ (a symbol sparse defines),
> > for two new fdt types: fdt32_t and fdt64_t, and subsequently
> > silences warnings emitted by sparse when parsing libfdt.
> 
> Should libfdt patches be committed to the main dtc repository (which I
> assume is also upstream for libfdt?) rather than U-Boot first?

that's what this is meant to be: a patch for dtc, cc: u-boot, since
it was originally a patch to u-boot's copy.  Fyi, dtc sumbission
guidelines dictate To: jdl, Cc: devicetree-discuss...

> Otherwise, if we want to bring in a new libfdt from upstream, that would
> trash all the U-Boot-specific changes in U-Boot's copy of libfdt.

right, this patch is for the dtc.  I assume gvb will pull it into
u-boot once it's applied.

Kim

Re: [PATCH] libfdt: introduce fdt type annotation for use by endian checkers

[David Gibson]

On Tue, Oct 30, 2012 at 04:57:54PM -0500, Kim Phillips wrote:
> Projects such as linux and u-boot run sparse on libfdt.  libfdt
> contains the notion of endianness via usage of endian conversion
> functions such as fdt32_to_cpu.  As such, in order to pass endian
> checks, libfdt has to annotate its fdt variables as big endian.
> This patch does that ifdef __CHECKER__ (a symbol sparse defines),
> for two new fdt types: fdt32_t and fdt64_t, and subsequently
> silences warnings emitted by sparse when parsing libfdt.
> 
> Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
> note: wasn't sure whether to introduce the new fdt32 types, or
> just have libfdt use __be32 directly.

I prefer having the fdt32 types, to match the cpu_to_fdt32() and so
forth functions we already use.

So I like this in principle, but a couple of nits.

First, I'd really like to see an accompanying patch that adds targets
to the dtc makefiles to run sparse over the sources.  I couldn't
really test this, because I couldn't figure out quite what options I
needed to invoke sparse with to get it to work properly.

And I'd also like to see the default libfdt_env.h updated to supply
the necessary sparse stuff - including the necessary __force casts in
its byteswap functions.

[snip]
> diff --git a/libfdt/fdt.h b/libfdt/fdt.h
> index 48ccfd9..0d9c856 100644
> --- a/libfdt/fdt.h
> +++ b/libfdt/fdt.h
> @@ -3,46 +3,54 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +#ifdef __CHECKER__

So, I'd prefer not to use __CHECKER__ directly here.  I'd rather we
defined a new specific symbol, that libfdt_env.h can set based on
__CHECKER__ if it wants.  Let's say _FDT_SPARSE The reason is
because..

> +typedef __be32 fdt32_t;
> +typedef __be64 fdt64_t;

..just running sparse does *not* immediately give you __be32 and
__be64 types - those have to be defined in terms of
__attribute__((bitwise)) and whatnot.  Your uboot env probably does
that already, but the default libfdt_env.h certainly doesn't.
Effectively _FDT_SPARSE is sayint two things, first that we're
compiling under sparse, but also that we have suitably defined endian
types in the environment.

Actually, given that libfdt_env.h is already required to provide the
cpu_to_fdt32() and so forth macros, I think it's slightly neater to
just require it to directly supply the fdt32_t etc. types when it
defines _FDT_SPARSE as well, rather than defining them in terms of
__beXX here then in terms of the attributes in the environment.

-- 
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

[PATCH 3/4 v2] dtc/libfdt: introduce fdt types for annotation by endian checkers

[Kim Phillips]

Projects such as linux and u-boot run sparse on libfdt.  libfdt
contains the notion of endianness via usage of endian conversion
functions such as fdt32_to_cpu.  As such, in order to pass endian
checks, libfdt has to annotate its fdt variables such that sparse
can warn when mixing bitwise and regular integers.  This patch adds
these new fdtXX_t types and, ifdef __CHECKER__ (a symbol sparse
defines), includes the bitwise annotation.

Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
---
v2:
adds bitwise awareness: determine host endianness manually, and
annotate swabs with __force in fdtXX_to_cpu and cpu_to_fdtXX
conversion functions (the inline endian condition checks are
optimized out at compile time).  This allows us to be able to check
libfdt bitwise conversions with sparse by building dtc with make
CC=cgcc. v2 also moves fdt32 definitions from fdt.h to libfdt_env.h
and changes fdt32 definitions to use __bitwise instead of __be32. No
separate _FDT_SPARSE was introduced because there's no need: using
__CHECKER__ directly is valid because it only occurs once, and in
libfdt_env.h.
In addition, the libfdt sparse fixes have been moved to a subsequent
patch.

I hope this addresses all your comments, David.

 libfdt/fdt.h        | 42 +++++++++++++++++-----------------
 libfdt/libfdt_env.h | 66 +++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 73 insertions(+), 35 deletions(-)

diff --git a/libfdt/fdt.h b/libfdt/fdt.h
index 48ccfd9..45dd134 100644
--- a/libfdt/fdt.h
+++ b/libfdt/fdt.h
@@ -4,45 +4,45 @@
 #ifndef __ASSEMBLY__
 
 struct fdt_header {
-	uint32_t magic;			 /* magic word FDT_MAGIC */
-	uint32_t totalsize;		 /* total size of DT block */
-	uint32_t off_dt_struct;		 /* offset to structure */
-	uint32_t off_dt_strings;	 /* offset to strings */
-	uint32_t off_mem_rsvmap;	 /* offset to memory reserve map */
-	uint32_t version;		 /* format version */
-	uint32_t last_comp_version;	 /* last compatible version */
+	fdt32_t magic;			 /* magic word FDT_MAGIC */
+	fdt32_t totalsize;		 /* total size of DT block */
+	fdt32_t off_dt_struct;		 /* offset to structure */
+	fdt32_t off_dt_strings;		 /* offset to strings */
+	fdt32_t off_mem_rsvmap;		 /* offset to memory reserve map */
+	fdt32_t version;		 /* format version */
+	fdt32_t last_comp_version;	 /* last compatible version */
 
 	/* version 2 fields below */
-	uint32_t boot_cpuid_phys;	 /* Which physical CPU id we're
+	fdt32_t boot_cpuid_phys;	 /* Which physical CPU id we're
 					    booting on */
 	/* version 3 fields below */
-	uint32_t size_dt_strings;	 /* size of the strings block */
+	fdt32_t size_dt_strings;	 /* size of the strings block */
 
 	/* version 17 fields below */
-	uint32_t size_dt_struct;	 /* size of the structure block */
+	fdt32_t size_dt_struct;		 /* size of the structure block */
 };
 
 struct fdt_reserve_entry {
-	uint64_t address;
-	uint64_t size;
+	fdt64_t address;
+	fdt64_t size;
 };
 
 struct fdt_node_header {
-	uint32_t tag;
+	fdt32_t tag;
 	char name[0];
 };
 
 struct fdt_property {
-	uint32_t tag;
-	uint32_t len;
-	uint32_t nameoff;
+	fdt32_t tag;
+	fdt32_t len;
+	fdt32_t nameoff;
 	char data[0];
 };
 
 #endif /* !__ASSEMBLY */
 
 #define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
-#define FDT_TAGSIZE	sizeof(uint32_t)
+#define FDT_TAGSIZE	sizeof(fdt32_t)
 
 #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
 #define FDT_END_NODE	0x2		/* End node */
@@ -51,10 +51,10 @@ struct fdt_property {
 #define FDT_NOP		0x4		/* nop */
 #define FDT_END		0x9
 
-#define FDT_V1_SIZE	(7*sizeof(uint32_t))
-#define FDT_V2_SIZE	(FDT_V1_SIZE + sizeof(uint32_t))
-#define FDT_V3_SIZE	(FDT_V2_SIZE + sizeof(uint32_t))
+#define FDT_V1_SIZE	(7*sizeof(fdt32_t))
+#define FDT_V2_SIZE	(FDT_V1_SIZE + sizeof(fdt32_t))
+#define FDT_V3_SIZE	(FDT_V2_SIZE + sizeof(fdt32_t))
 #define FDT_V16_SIZE	FDT_V3_SIZE
-#define FDT_V17_SIZE	(FDT_V16_SIZE + sizeof(uint32_t))
+#define FDT_V17_SIZE	(FDT_V16_SIZE + sizeof(fdt32_t))
 
 #endif /* _FDT_H */
diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h
index 213d7fb..302d5cb 100644
--- a/libfdt/libfdt_env.h
+++ b/libfdt/libfdt_env.h
@@ -5,25 +5,63 @@
 #include <stdint.h>
 #include <string.h>
 
+#ifdef __CHECKER__
+#define __force __attribute__((force))
+#define __bitwise __attribute__((bitwise))
+typedef uint16_t __bitwise fdt16_t;
+typedef uint32_t __bitwise fdt32_t;
+typedef uint64_t __bitwise fdt64_t;
+#else
+#define __force
+#define __bitwise
+typedef uint16_t fdt16_t;
+typedef uint32_t fdt32_t;
+typedef uint64_t fdt64_t;
+#endif
+
 #define EXTRACT_BYTE(n)	((unsigned long long)((uint8_t *)&x)[n])
-static inline uint16_t fdt16_to_cpu(uint16_t x)
-{
-	return (EXTRACT_BYTE(0) << 8) | EXTRACT_BYTE(1);
-}
-#define cpu_to_fdt16(x) fdt16_to_cpu(x)
+#define __SWAB16X ((EXTRACT_BYTE(0) << 8) | EXTRACT_BYTE(1))
+#define __SWAB32X ((EXTRACT_BYTE(0) << 24) | (EXTRACT_BYTE(1) << 16) | (EXTRACT_BYTE(2) << 8) | EXTRACT_BYTE(3))
+#define __SWAB64X ((EXTRACT_BYTE(0) << 56) | (EXTRACT_BYTE(1) << 48) | (EXTRACT_BYTE(2) << 40) | (EXTRACT_BYTE(3) << 32) \
+		  | (EXTRACT_BYTE(4) << 24) | (EXTRACT_BYTE(5) << 16) | (EXTRACT_BYTE(6) << 8) | EXTRACT_BYTE(7))
 
-static inline uint32_t fdt32_to_cpu(uint32_t x)
-{
-	return (EXTRACT_BYTE(0) << 24) | (EXTRACT_BYTE(1) << 16) | (EXTRACT_BYTE(2) << 8) | EXTRACT_BYTE(3);
+/*
+ * determine host endianness:
+ * *__first_byte is 0x11 on big endian systems
+ * *__first_byte is 0x44 on little endian systems
+ */
+static const uint32_t __native = 0x11223344u;
+static const uint8_t *__first_byte = (const uint8_t *)&__native;
+
+#define DEF_FDT_TO_CPU(bits) \
+static inline uint##bits##_t fdt##bits##_to_cpu(fdt##bits##_t x) \
+{ \
+	if (*__first_byte == 0x11) \
+		return (__force uint##bits##_t)x; \
+	else \
+		return (__force uint##bits##_t)__SWAB##bits##X; \
 }
-#define cpu_to_fdt32(x) fdt32_to_cpu(x)
+DEF_FDT_TO_CPU(16)
+DEF_FDT_TO_CPU(32)
+DEF_FDT_TO_CPU(64)
 
-static inline uint64_t fdt64_to_cpu(uint64_t x)
-{
-	return (EXTRACT_BYTE(0) << 56) | (EXTRACT_BYTE(1) << 48) | (EXTRACT_BYTE(2) << 40) | (EXTRACT_BYTE(3) << 32)
-		| (EXTRACT_BYTE(4) << 24) | (EXTRACT_BYTE(5) << 16) | (EXTRACT_BYTE(6) << 8) | EXTRACT_BYTE(7);
+#define DEF_CPU_TO_FDT(bits) \
+static inline fdt##bits##_t cpu_to_fdt##bits(uint##bits##_t x) \
+{ \
+	if (*__first_byte == 0x11) \
+		return (__force fdt##bits##_t)x; \
+	else \
+		return (__force fdt##bits##_t)__SWAB##bits##X; \
 }
-#define cpu_to_fdt64(x) fdt64_to_cpu(x)
+DEF_CPU_TO_FDT(16)
+DEF_CPU_TO_FDT(32)
+DEF_CPU_TO_FDT(64)
+
+#undef DEF_FDT_TO_CPU
+#undef DEF_CPU_TO_FDT
+#undef __SWAB64X
+#undef __SWAB32X
+#undef __SWAB16X
 #undef EXTRACT_BYTE
 
 #endif /* _LIBFDT_ENV_H */
-- 
1.8.0

Re: [PATCH 3/4 v2] dtc/libfdt: introduce fdt types for annotation by endian checkers

[Jon Loeliger]

Hi Kim,

> 
> I hope this addresses all your comments, David.

Which is why David didn't see this patch earlier. :-)


> index 213d7fb..302d5cb 100644
> --- a/libfdt/libfdt_env.h
> +++ b/libfdt/libfdt_env.h
> @@ -5,25 +5,63 @@
>  #include <stdint.h>
>  #include <string.h>
>  
> +#ifdef __CHECKER__
> +#define __force __attribute__((force))
> +#define __bitwise __attribute__((bitwise))
> +typedef uint16_t __bitwise fdt16_t;
> +typedef uint32_t __bitwise fdt32_t;
> +typedef uint64_t __bitwise fdt64_t;
> +#else
> +#define __force
> +#define __bitwise
> +typedef uint16_t fdt16_t;
> +typedef uint32_t fdt32_t;
> +typedef uint64_t fdt64_t;
> +#endif

Would this be simpler/better?

    #ifdef __CHECKER__
    #define __force __attribute__((force))
    #define __bitwise __attribute__((bitwise))
    #else
    #define __force
    #define __bitwise
    #endif

    typedef uint16_t __bitwise fdt16_t;
    typedef uint32_t __bitwise fdt32_t;
    typedef uint64_t __bitwise fdt64_t;



>  #define EXTRACT_BYTE(n)	((unsigned long long)((uint8_t *)&x)[n])
> -static inline uint16_t fdt16_to_cpu(uint16_t x)
> -{
> -	return (EXTRACT_BYTE(0) << 8) | EXTRACT_BYTE(1);
> -}
> -#define cpu_to_fdt16(x) fdt16_to_cpu(x)
> +#define __SWAB16X ((EXTRACT_BYTE(0) << 8) | EXTRACT_BYTE(1))
> +#define __SWAB32X ((EXTRACT_BYTE(0) << 24) | (EXTRACT_BYTE(1) << 16) | (EXTRACT_BYTE(2) <<
>  8) | EXTRACT_BYTE(3))
> +#define __SWAB64X ((EXTRACT_BYTE(0) << 56) | (EXTRACT_BYTE(1) << 48) | (EXTRACT_BYTE(2) <<
>  40) | (EXTRACT_BYTE(3) << 32) \
> +		  | (EXTRACT_BYTE(4) << 24) | (EXTRACT_BYTE(5) << 16) | (EXTRACT_BYTE(6) << 8
> ) | EXTRACT_BYTE(7))

I dislike function-like macros that grab global names.
Instead, can we re-work 'x' in as a parameter:

    >  #define EXTRACT_BYTE(x, n)	((unsigned long long)((uint8_t *)&x)[n])


> +#define DEF_FDT_TO_CPU(bits) \
> +static inline uint##bits##_t fdt##bits##_to_cpu(fdt##bits##_t x) \
> +{ \
> +	if (*__first_byte == 0x11) \
> +		return (__force uint##bits##_t)x; \
> +	else \
> +		return (__force uint##bits##_t)__SWAB##bits##X; \
>  }

Case check that last X...

> -static inline uint64_t fdt64_to_cpu(uint64_t x)
> -{
> -	return (EXTRACT_BYTE(0) << 56) | (EXTRACT_BYTE(1) << 48) | (EXTRACT_BYTE(2) << 40) | 
> (EXTRACT_BYTE(3) << 32)
> -		| (EXTRACT_BYTE(4) << 24) | (EXTRACT_BYTE(5) << 16) | (EXTRACT_BYTE(6) << 8) 
> | EXTRACT_BYTE(7);
> +#define DEF_CPU_TO_FDT(bits) \
> +static inline fdt##bits##_t cpu_to_fdt##bits(uint##bits##_t x) \
> +{ \
> +	if (*__first_byte == 0x11) \
> +		return (__force fdt##bits##_t)x; \
> +	else \
> +		return (__force fdt##bits##_t)__SWAB##bits##X; \
>  }

...and that one.

Thanks,
jdl

[PATCH v2 1/4] dtc/tests: don't include fdt.h prior to libfdt.h

[Kim Phillips]

tests will need fdt type definitions provided in a subsequent patch
to libfdt_env.h.  Since libfdt.h includes libfdt_env.h in the right
order anyway, just remove the fdt.h include.

Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
---
v2: added David's Acked-by.

 tests/add_subnode_with_nops.c     | 1 -
 tests/appendprop1.c               | 1 -
 tests/appendprop2.c               | 1 -
 tests/asm_tree_dump.c             | 1 -
 tests/boot-cpuid.c                | 1 -
 tests/char_literal.c              | 1 -
 tests/del_node.c                  | 1 -
 tests/del_property.c              | 1 -
 tests/dtb_reverse.c               | 1 -
 tests/dtbs_equal_ordered.c        | 1 -
 tests/dtbs_equal_unordered.c      | 1 -
 tests/dumptrees.c                 | 2 --
 tests/extra-terminating-null.c    | 1 -
 tests/find_property.c             | 1 -
 tests/get_alias.c                 | 1 -
 tests/get_mem_rsv.c               | 1 -
 tests/get_name.c                  | 1 -
 tests/get_path.c                  | 1 -
 tests/get_phandle.c               | 1 -
 tests/getprop.c                   | 1 -
 tests/incbin.c                    | 1 -
 tests/integer-expressions.c       | 1 -
 tests/mangle-layout.c             | 1 -
 tests/move_and_save.c             | 1 -
 tests/node_check_compatible.c     | 1 -
 tests/node_offset_by_compatible.c | 1 -
 tests/node_offset_by_phandle.c    | 1 -
 tests/node_offset_by_prop_value.c | 1 -
 tests/nop_node.c                  | 1 -
 tests/nop_property.c              | 1 -
 tests/nopulate.c                  | 1 -
 tests/notfound.c                  | 1 -
 tests/open_pack.c                 | 1 -
 tests/parent_offset.c             | 1 -
 tests/path-references.c           | 1 -
 tests/path_offset.c               | 1 -
 tests/path_offset_aliases.c       | 1 -
 tests/phandle_format.c            | 1 -
 tests/propname_escapes.c          | 1 -
 tests/references.c                | 1 -
 tests/root_node.c                 | 1 -
 tests/rw_tree1.c                  | 1 -
 tests/set_name.c                  | 1 -
 tests/setprop.c                   | 1 -
 tests/setprop_inplace.c           | 1 -
 tests/sized_cells.c               | 1 -
 tests/string_escapes.c            | 1 -
 tests/subnode_offset.c            | 1 -
 tests/supernode_atdepth_offset.c  | 1 -
 tests/sw_tree1.c                  | 1 -
 tests/truncated_property.c        | 1 -
 tests/utilfdt_test.c              | 1 -
 tests/value-labels.c              | 1 -
 53 files changed, 54 deletions(-)

diff --git a/tests/add_subnode_with_nops.c b/tests/add_subnode_with_nops.c
index 4fb8f02..95ddf6a 100644
--- a/tests/add_subnode_with_nops.c
+++ b/tests/add_subnode_with_nops.c
@@ -24,7 +24,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/appendprop1.c b/tests/appendprop1.c
index d716f7a..9d6b3ad 100644
--- a/tests/appendprop1.c
+++ b/tests/appendprop1.c
@@ -24,7 +24,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/appendprop2.c b/tests/appendprop2.c
index 7eb243d..ca1446c 100644
--- a/tests/appendprop2.c
+++ b/tests/appendprop2.c
@@ -24,7 +24,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/asm_tree_dump.c b/tests/asm_tree_dump.c
index 5ff5087..bd12eda 100644
--- a/tests/asm_tree_dump.c
+++ b/tests/asm_tree_dump.c
@@ -26,7 +26,6 @@
 
 #include <dlfcn.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/boot-cpuid.c b/tests/boot-cpuid.c
index 7b5433d..ca39f4b 100644
--- a/tests/boot-cpuid.c
+++ b/tests/boot-cpuid.c
@@ -21,7 +21,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/char_literal.c b/tests/char_literal.c
index 150f2a0..d7a4773 100644
--- a/tests/char_literal.c
+++ b/tests/char_literal.c
@@ -23,7 +23,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/del_node.c b/tests/del_node.c
index afad502..45cb060 100644
--- a/tests/del_node.c
+++ b/tests/del_node.c
@@ -24,7 +24,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/del_property.c b/tests/del_property.c
index 449eca6..42fd7cb 100644
--- a/tests/del_property.c
+++ b/tests/del_property.c
@@ -24,7 +24,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/dtb_reverse.c b/tests/dtb_reverse.c
index 25e1eef..527fd71 100644
--- a/tests/dtb_reverse.c
+++ b/tests/dtb_reverse.c
@@ -24,7 +24,6 @@
 #include <stdint.h>
 #include <limits.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/dtbs_equal_ordered.c b/tests/dtbs_equal_ordered.c
index 1db25f4..12495de 100644
--- a/tests/dtbs_equal_ordered.c
+++ b/tests/dtbs_equal_ordered.c
@@ -23,7 +23,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/dtbs_equal_unordered.c b/tests/dtbs_equal_unordered.c
index df53318..20b4356 100644
--- a/tests/dtbs_equal_unordered.c
+++ b/tests/dtbs_equal_unordered.c
@@ -24,7 +24,6 @@
 #include <stdint.h>
 #include <limits.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/dumptrees.c b/tests/dumptrees.c
index fa1f563..bebf553 100644
--- a/tests/dumptrees.c
+++ b/tests/dumptrees.c
@@ -25,9 +25,7 @@
 #include <fcntl.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
-#include <libfdt_env.h>
 
 #include "testdata.h"
 
diff --git a/tests/extra-terminating-null.c b/tests/extra-terminating-null.c
index 8a2043f..dc1fe89 100644
--- a/tests/extra-terminating-null.c
+++ b/tests/extra-terminating-null.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/find_property.c b/tests/find_property.c
index 74a6965..4dc3030 100644
--- a/tests/find_property.c
+++ b/tests/find_property.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/get_alias.c b/tests/get_alias.c
index 1e0faf4..5060795 100644
--- a/tests/get_alias.c
+++ b/tests/get_alias.c
@@ -23,7 +23,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/get_mem_rsv.c b/tests/get_mem_rsv.c
index 554c788..1812639 100644
--- a/tests/get_mem_rsv.c
+++ b/tests/get_mem_rsv.c
@@ -23,7 +23,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/get_name.c b/tests/get_name.c
index 0262a12..c6ca9f9 100644
--- a/tests/get_name.c
+++ b/tests/get_name.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/get_path.c b/tests/get_path.c
index 1e05f7c..7352976 100644
--- a/tests/get_path.c
+++ b/tests/get_path.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/get_phandle.c b/tests/get_phandle.c
index 5735733..2079591 100644
--- a/tests/get_phandle.c
+++ b/tests/get_phandle.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/getprop.c b/tests/getprop.c
index 239856e..6255bad 100644
--- a/tests/getprop.c
+++ b/tests/getprop.c
@@ -23,7 +23,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/incbin.c b/tests/incbin.c
index 76d8626..4100ba0 100644
--- a/tests/incbin.c
+++ b/tests/incbin.c
@@ -23,7 +23,6 @@
 #include <stdint.h>
 #include <errno.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/integer-expressions.c b/tests/integer-expressions.c
index 5ba1566..57e2ff6 100644
--- a/tests/integer-expressions.c
+++ b/tests/integer-expressions.c
@@ -25,7 +25,6 @@
 #include <errno.h>
 
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/mangle-layout.c b/tests/mangle-layout.c
index 3b19788..a76e51e 100644
--- a/tests/mangle-layout.c
+++ b/tests/mangle-layout.c
@@ -24,7 +24,6 @@
 #include <limits.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/move_and_save.c b/tests/move_and_save.c
index 410ccb3..393b60a 100644
--- a/tests/move_and_save.c
+++ b/tests/move_and_save.c
@@ -24,7 +24,6 @@
 #include <limits.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/node_check_compatible.c b/tests/node_check_compatible.c
index 23abbf5..4bdf091 100644
--- a/tests/node_check_compatible.c
+++ b/tests/node_check_compatible.c
@@ -23,7 +23,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/node_offset_by_compatible.c b/tests/node_offset_by_compatible.c
index 2317930..f62b591 100644
--- a/tests/node_offset_by_compatible.c
+++ b/tests/node_offset_by_compatible.c
@@ -23,7 +23,6 @@
 #include <stdint.h>
 #include <stdarg.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/node_offset_by_phandle.c b/tests/node_offset_by_phandle.c
index a8442f1..becff0f 100644
--- a/tests/node_offset_by_phandle.c
+++ b/tests/node_offset_by_phandle.c
@@ -23,7 +23,6 @@
 #include <stdint.h>
 #include <stdarg.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/node_offset_by_prop_value.c b/tests/node_offset_by_prop_value.c
index 0f2a345..9212a4e 100644
--- a/tests/node_offset_by_prop_value.c
+++ b/tests/node_offset_by_prop_value.c
@@ -23,7 +23,6 @@
 #include <stdint.h>
 #include <stdarg.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/nop_node.c b/tests/nop_node.c
index ea3a18f..c316444 100644
--- a/tests/nop_node.c
+++ b/tests/nop_node.c
@@ -24,7 +24,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/nop_property.c b/tests/nop_property.c
index e6ef4d9..644b0a6 100644
--- a/tests/nop_property.c
+++ b/tests/nop_property.c
@@ -24,7 +24,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/nopulate.c b/tests/nopulate.c
index 3cbbe21..cd79872 100644
--- a/tests/nopulate.c
+++ b/tests/nopulate.c
@@ -24,7 +24,6 @@
 #include <limits.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/notfound.c b/tests/notfound.c
index 4d55b88..dc623d6 100644
--- a/tests/notfound.c
+++ b/tests/notfound.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/open_pack.c b/tests/open_pack.c
index 0a5a3fc..407ef6c 100644
--- a/tests/open_pack.c
+++ b/tests/open_pack.c
@@ -24,7 +24,6 @@
 #include <limits.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/parent_offset.c b/tests/parent_offset.c
index e7affcc..d4ab3cf 100644
--- a/tests/parent_offset.c
+++ b/tests/parent_offset.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/path-references.c b/tests/path-references.c
index 9f363b3..0746b3f 100644
--- a/tests/path-references.c
+++ b/tests/path-references.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/path_offset.c b/tests/path_offset.c
index d3e1f8e..4e5b7a1 100644
--- a/tests/path_offset.c
+++ b/tests/path_offset.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/path_offset_aliases.c b/tests/path_offset_aliases.c
index 3682da4..78d5a46 100644
--- a/tests/path_offset_aliases.c
+++ b/tests/path_offset_aliases.c
@@ -23,7 +23,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/phandle_format.c b/tests/phandle_format.c
index 7e4d816..5874ae7 100644
--- a/tests/phandle_format.c
+++ b/tests/phandle_format.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/propname_escapes.c b/tests/propname_escapes.c
index 3aec28f..e91bd99 100644
--- a/tests/propname_escapes.c
+++ b/tests/propname_escapes.c
@@ -24,7 +24,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/references.c b/tests/references.c
index b20f21f..c9d05a2 100644
--- a/tests/references.c
+++ b/tests/references.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/root_node.c b/tests/root_node.c
index 3f47829..58aebf6 100644
--- a/tests/root_node.c
+++ b/tests/root_node.c
@@ -23,7 +23,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/rw_tree1.c b/tests/rw_tree1.c
index 103a24d..efd4718 100644
--- a/tests/rw_tree1.c
+++ b/tests/rw_tree1.c
@@ -24,7 +24,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/set_name.c b/tests/set_name.c
index 5d1149e..9861587 100644
--- a/tests/set_name.c
+++ b/tests/set_name.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/setprop.c b/tests/setprop.c
index 9f2bc88..d089f8d 100644
--- a/tests/setprop.c
+++ b/tests/setprop.c
@@ -24,7 +24,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/setprop_inplace.c b/tests/setprop_inplace.c
index 82d8951..daef182 100644
--- a/tests/setprop_inplace.c
+++ b/tests/setprop_inplace.c
@@ -25,7 +25,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/sized_cells.c b/tests/sized_cells.c
index 847ec96..94da03b 100644
--- a/tests/sized_cells.c
+++ b/tests/sized_cells.c
@@ -23,7 +23,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/string_escapes.c b/tests/string_escapes.c
index 30eb6a8..8cdee4b 100644
--- a/tests/string_escapes.c
+++ b/tests/string_escapes.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/subnode_offset.c b/tests/subnode_offset.c
index e58c192..231fcb5 100644
--- a/tests/subnode_offset.c
+++ b/tests/subnode_offset.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/supernode_atdepth_offset.c b/tests/supernode_atdepth_offset.c
index 73f41ae..43e120d 100644
--- a/tests/supernode_atdepth_offset.c
+++ b/tests/supernode_atdepth_offset.c
@@ -22,7 +22,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/sw_tree1.c b/tests/sw_tree1.c
index 5c71414..8eb6e5f 100644
--- a/tests/sw_tree1.c
+++ b/tests/sw_tree1.c
@@ -24,7 +24,6 @@
 #include <ctype.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/truncated_property.c b/tests/truncated_property.c
index 56daa22..f820d99 100644
--- a/tests/truncated_property.c
+++ b/tests/truncated_property.c
@@ -23,7 +23,6 @@
 #include <string.h>
 #include <stdint.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
diff --git a/tests/utilfdt_test.c b/tests/utilfdt_test.c
index 36b4aa5..274c3d6 100644
--- a/tests/utilfdt_test.c
+++ b/tests/utilfdt_test.c
@@ -24,7 +24,6 @@
 #include <stdint.h>
 #include <stdarg.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 #include <util.h>
 
diff --git a/tests/value-labels.c b/tests/value-labels.c
index abe2721..dcf2059 100644
--- a/tests/value-labels.c
+++ b/tests/value-labels.c
@@ -26,7 +26,6 @@
 
 #include <dlfcn.h>
 
-#include <fdt.h>
 #include <libfdt.h>
 
 #include "tests.h"
-- 
1.8.0

[PATCH v2 2/4] dtc/fdtdump: include libfdt_env.h prior to fdt.h

[Kim Phillips]

in order to get the upcoming fdt type definitions.

Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
Acked-by: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
---
v2: added David's Acked-by.

 fdtdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fdtdump.c b/fdtdump.c
index 207a46d..8a7ae72 100644
--- a/fdtdump.c
+++ b/fdtdump.c
@@ -8,8 +8,8 @@
 #include <string.h>
 #include <ctype.h>
 
-#include <fdt.h>
 #include <libfdt_env.h>
+#include <fdt.h>
 
 #include "util.h"
 
-- 
1.8.0

[PATCH v3 3/4] dtc/libfdt: introduce fdt types for annotation by endian checkers

[Kim Phillips]

Projects such as linux and u-boot run sparse on libfdt.  libfdt
contains the notion of endianness via usage of endian conversion
functions such as fdt32_to_cpu.  As such, in order to pass endian
checks, libfdt has to annotate its fdt variables such that sparse
can warn when mixing bitwise and regular integers.  This patch adds
these new fdtXX_t types and, ifdef __CHECKER__ (a symbol sparse
defines), includes the bitwise annotation.

Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
---
v2:
adds bitwise awareness: determine host endianness manually, and
annotate swabs with __force in fdtXX_to_cpu and cpu_to_fdtXX
conversion functions (the inline endian condition checks are
optimized out at compile time).  This allows us to be able to check
libfdt bitwise conversions with sparse by building dtc with make
CC=cgcc. v2 also moves fdt32 definitions from fdt.h to libfdt_env.h
and changes fdt32 definitions to use __bitwise instead of __be32. No
separate _FDT_SPARSE was introduced because there's no need: using
__CHECKER__ directly is valid because it only occurs once, and in
libfdt_env.h.
In addition, the libfdt sparse fixes have been moved to a subsequent
patch.

v3:  address comments from jdl:
o single set of fdt typedefs, since __bitwise is not defined if !CHECKER
o re-work EXTRACT_BYTE to take 'x' as a parameter, not a global
o SWAB function macros converted to take lowercase 'x' as a parameter,
  not a global

 libfdt/fdt.h        | 42 ++++++++++++++++----------------
 libfdt/libfdt_env.h | 69 +++++++++++++++++++++++++++++++++++++++++------------
 2 files changed, 75 insertions(+), 36 deletions(-)

diff --git a/libfdt/fdt.h b/libfdt/fdt.h
index 48ccfd9..45dd134 100644
--- a/libfdt/fdt.h
+++ b/libfdt/fdt.h
@@ -4,45 +4,45 @@
 #ifndef __ASSEMBLY__
 
 struct fdt_header {
-	uint32_t magic;			 /* magic word FDT_MAGIC */
-	uint32_t totalsize;		 /* total size of DT block */
-	uint32_t off_dt_struct;		 /* offset to structure */
-	uint32_t off_dt_strings;	 /* offset to strings */
-	uint32_t off_mem_rsvmap;	 /* offset to memory reserve map */
-	uint32_t version;		 /* format version */
-	uint32_t last_comp_version;	 /* last compatible version */
+	fdt32_t magic;			 /* magic word FDT_MAGIC */
+	fdt32_t totalsize;		 /* total size of DT block */
+	fdt32_t off_dt_struct;		 /* offset to structure */
+	fdt32_t off_dt_strings;		 /* offset to strings */
+	fdt32_t off_mem_rsvmap;		 /* offset to memory reserve map */
+	fdt32_t version;		 /* format version */
+	fdt32_t last_comp_version;	 /* last compatible version */
 
 	/* version 2 fields below */
-	uint32_t boot_cpuid_phys;	 /* Which physical CPU id we're
+	fdt32_t boot_cpuid_phys;	 /* Which physical CPU id we're
 					    booting on */
 	/* version 3 fields below */
-	uint32_t size_dt_strings;	 /* size of the strings block */
+	fdt32_t size_dt_strings;	 /* size of the strings block */
 
 	/* version 17 fields below */
-	uint32_t size_dt_struct;	 /* size of the structure block */
+	fdt32_t size_dt_struct;		 /* size of the structure block */
 };
 
 struct fdt_reserve_entry {
-	uint64_t address;
-	uint64_t size;
+	fdt64_t address;
+	fdt64_t size;
 };
 
 struct fdt_node_header {
-	uint32_t tag;
+	fdt32_t tag;
 	char name[0];
 };
 
 struct fdt_property {
-	uint32_t tag;
-	uint32_t len;
-	uint32_t nameoff;
+	fdt32_t tag;
+	fdt32_t len;
+	fdt32_t nameoff;
 	char data[0];
 };
 
 #endif /* !__ASSEMBLY */
 
 #define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
-#define FDT_TAGSIZE	sizeof(uint32_t)
+#define FDT_TAGSIZE	sizeof(fdt32_t)
 
 #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
 #define FDT_END_NODE	0x2		/* End node */
@@ -51,10 +51,10 @@ struct fdt_property {
 #define FDT_NOP		0x4		/* nop */
 #define FDT_END		0x9
 
-#define FDT_V1_SIZE	(7*sizeof(uint32_t))
-#define FDT_V2_SIZE	(FDT_V1_SIZE + sizeof(uint32_t))
-#define FDT_V3_SIZE	(FDT_V2_SIZE + sizeof(uint32_t))
+#define FDT_V1_SIZE	(7*sizeof(fdt32_t))
+#define FDT_V2_SIZE	(FDT_V1_SIZE + sizeof(fdt32_t))
+#define FDT_V3_SIZE	(FDT_V2_SIZE + sizeof(fdt32_t))
 #define FDT_V16_SIZE	FDT_V3_SIZE
-#define FDT_V17_SIZE	(FDT_V16_SIZE + sizeof(uint32_t))
+#define FDT_V17_SIZE	(FDT_V16_SIZE + sizeof(fdt32_t))
 
 #endif /* _FDT_H */
diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h
index 213d7fb..f0d97b9 100644
--- a/libfdt/libfdt_env.h
+++ b/libfdt/libfdt_env.h
@@ -5,25 +5,64 @@
 #include <stdint.h>
 #include <string.h>
 
-#define EXTRACT_BYTE(n)	((unsigned long long)((uint8_t *)&x)[n])
-static inline uint16_t fdt16_to_cpu(uint16_t x)
-{
-	return (EXTRACT_BYTE(0) << 8) | EXTRACT_BYTE(1);
-}
-#define cpu_to_fdt16(x) fdt16_to_cpu(x)
+#ifdef __CHECKER__
+#define __force __attribute__((force))
+#define __bitwise __attribute__((bitwise))
+#else
+#define __force
+#define __bitwise
+#endif
+
+typedef uint16_t __bitwise fdt16_t;
+typedef uint32_t __bitwise fdt32_t;
+typedef uint64_t __bitwise fdt64_t;
+
+#define EXTRACT_BYTE(x, n) ((unsigned long long)((uint8_t *)&x)[n])
+#define __SWAB16(x) ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
+#define __SWAB32(x) ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
+		     (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
+#define __SWAB64(x) ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
+		     (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
+		     (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
+		     (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
 
-static inline uint32_t fdt32_to_cpu(uint32_t x)
-{
-	return (EXTRACT_BYTE(0) << 24) | (EXTRACT_BYTE(1) << 16) | (EXTRACT_BYTE(2) << 8) | EXTRACT_BYTE(3);
+/*
+ * determine host endianness:
+ * *__first_byte is 0x11 on big endian systems
+ * *__first_byte is 0x44 on little endian systems
+ */
+static const uint32_t __native = 0x11223344u;
+static const uint8_t *__first_byte = (const uint8_t *)&__native;
+
+#define DEF_FDT_TO_CPU(bits) \
+static inline uint##bits##_t fdt##bits##_to_cpu(fdt##bits##_t x) \
+{ \
+	if (*__first_byte == 0x11) \
+		return (__force uint##bits##_t)x; \
+	else \
+		return (__force uint##bits##_t)__SWAB##bits(x); \
 }
-#define cpu_to_fdt32(x) fdt32_to_cpu(x)
+DEF_FDT_TO_CPU(16)
+DEF_FDT_TO_CPU(32)
+DEF_FDT_TO_CPU(64)
 
-static inline uint64_t fdt64_to_cpu(uint64_t x)
-{
-	return (EXTRACT_BYTE(0) << 56) | (EXTRACT_BYTE(1) << 48) | (EXTRACT_BYTE(2) << 40) | (EXTRACT_BYTE(3) << 32)
-		| (EXTRACT_BYTE(4) << 24) | (EXTRACT_BYTE(5) << 16) | (EXTRACT_BYTE(6) << 8) | EXTRACT_BYTE(7);
+#define DEF_CPU_TO_FDT(bits) \
+static inline fdt##bits##_t cpu_to_fdt##bits(uint##bits##_t x) \
+{ \
+	if (*__first_byte == 0x11) \
+		return (__force fdt##bits##_t)x; \
+	else \
+		return (__force fdt##bits##_t)__SWAB##bits(x); \
 }
-#define cpu_to_fdt64(x) fdt64_to_cpu(x)
+DEF_CPU_TO_FDT(16)
+DEF_CPU_TO_FDT(32)
+DEF_CPU_TO_FDT(64)
+
+#undef DEF_CPU_TO_FDT
+#undef DEF_FDT_TO_CPU
+#undef __SWAB64
+#undef __SWAB32
+#undef __SWAB16
 #undef EXTRACT_BYTE
 
 #endif /* _LIBFDT_ENV_H */
-- 
1.8.0

[PATCH v2 4/4] dtc/libfdt: uintXX_t to fdtXX_t conversion

[Kim Phillips]

Now that fdt types are defined and annotated, use them to
make sparse happy.

Signed-off-by: Kim Phillips <kim.phillips@freescale.com>
---
v2: reworded commit text

 libfdt/fdt.c     |  2 +-
 libfdt/fdt_ro.c  |  2 +-
 libfdt/fdt_rw.c  |  4 ++--
 libfdt/fdt_sw.c  |  4 ++--
 libfdt/fdt_wip.c |  2 +-
 libfdt/libfdt.h  | 32 ++++++++++++++++----------------
 6 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/libfdt/fdt.c b/libfdt/fdt.c
index e56833a..57faba3 100644
--- a/libfdt/fdt.c
+++ b/libfdt/fdt.c
@@ -92,7 +92,7 @@ const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
 
 uint32_t fdt_next_tag(const void *fdt, int startoffset, int *nextoffset)
 {
-	const uint32_t *tagp, *lenp;
+	const fdt32_t *tagp, *lenp;
 	uint32_t tag;
 	int offset = startoffset;
 	const char *p;
diff --git a/libfdt/fdt_ro.c b/libfdt/fdt_ro.c
index 02b6d68..42da2bd 100644
--- a/libfdt/fdt_ro.c
+++ b/libfdt/fdt_ro.c
@@ -322,7 +322,7 @@ const void *fdt_getprop(const void *fdt, int nodeoffset,
 
 uint32_t fdt_get_phandle(const void *fdt, int nodeoffset)
 {
-	const uint32_t *php;
+	const fdt32_t *php;
 	int len;
 
 	/* FIXME: This is a bit sub-optimal, since we potentially scan
diff --git a/libfdt/fdt_rw.c b/libfdt/fdt_rw.c
index 24437df..fdba618 100644
--- a/libfdt/fdt_rw.c
+++ b/libfdt/fdt_rw.c
@@ -339,7 +339,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
 	int nodelen;
 	int err;
 	uint32_t tag;
-	uint32_t *endtag;
+	fdt32_t *endtag;
 
 	FDT_RW_CHECK_HEADER(fdt);
 
@@ -366,7 +366,7 @@ int fdt_add_subnode_namelen(void *fdt, int parentoffset,
 	nh->tag = cpu_to_fdt32(FDT_BEGIN_NODE);
 	memset(nh->name, 0, FDT_TAGALIGN(namelen+1));
 	memcpy(nh->name, name, namelen);
-	endtag = (uint32_t *)((char *)nh + nodelen - FDT_TAGSIZE);
+	endtag = (fdt32_t *)((char *)nh + nodelen - FDT_TAGSIZE);
 	*endtag = cpu_to_fdt32(FDT_END_NODE);
 
 	return offset;
diff --git a/libfdt/fdt_sw.c b/libfdt/fdt_sw.c
index 55ebebf..f422754 100644
--- a/libfdt/fdt_sw.c
+++ b/libfdt/fdt_sw.c
@@ -153,7 +153,7 @@ int fdt_begin_node(void *fdt, const char *name)
 
 int fdt_end_node(void *fdt)
 {
-	uint32_t *en;
+	fdt32_t *en;
 
 	FDT_SW_CHECK_HEADER(fdt);
 
@@ -213,7 +213,7 @@ int fdt_property(void *fdt, const char *name, const void *val, int len)
 int fdt_finish(void *fdt)
 {
 	char *p = (char *)fdt;
-	uint32_t *end;
+	fdt32_t *end;
 	int oldstroffset, newstroffset;
 	uint32_t tag;
 	int offset, nextoffset;
diff --git a/libfdt/fdt_wip.c b/libfdt/fdt_wip.c
index 6025fa1..c5bbb68 100644
--- a/libfdt/fdt_wip.c
+++ b/libfdt/fdt_wip.c
@@ -74,7 +74,7 @@ int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
 
 static void _fdt_nop_region(void *start, int len)
 {
-	uint32_t *p;
+	fdt32_t *p;
 
 	for (p = start; (char *)p < ((char *)start + len); p++)
 		*p = cpu_to_fdt32(FDT_NOP);
diff --git a/libfdt/libfdt.h b/libfdt/libfdt.h
index 73f4975..8e57a06 100644
--- a/libfdt/libfdt.h
+++ b/libfdt/libfdt.h
@@ -882,8 +882,8 @@ int fdt_setprop_inplace(void *fdt, int nodeoffset, const char *name,
 static inline int fdt_setprop_inplace_u32(void *fdt, int nodeoffset,
 					  const char *name, uint32_t val)
 {
-	val = cpu_to_fdt32(val);
-	return fdt_setprop_inplace(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt32_t tmp = cpu_to_fdt32(val);
+	return fdt_setprop_inplace(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
@@ -917,8 +917,8 @@ static inline int fdt_setprop_inplace_u32(void *fdt, int nodeoffset,
 static inline int fdt_setprop_inplace_u64(void *fdt, int nodeoffset,
 					  const char *name, uint64_t val)
 {
-	val = cpu_to_fdt64(val);
-	return fdt_setprop_inplace(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt64_t tmp = cpu_to_fdt64(val);
+	return fdt_setprop_inplace(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
@@ -993,13 +993,13 @@ int fdt_begin_node(void *fdt, const char *name);
 int fdt_property(void *fdt, const char *name, const void *val, int len);
 static inline int fdt_property_u32(void *fdt, const char *name, uint32_t val)
 {
-	val = cpu_to_fdt32(val);
-	return fdt_property(fdt, name, &val, sizeof(val));
+	fdt32_t tmp = cpu_to_fdt32(val);
+	return fdt_property(fdt, name, &tmp, sizeof(tmp));
 }
 static inline int fdt_property_u64(void *fdt, const char *name, uint64_t val)
 {
-	val = cpu_to_fdt64(val);
-	return fdt_property(fdt, name, &val, sizeof(val));
+	fdt64_t tmp = cpu_to_fdt64(val);
+	return fdt_property(fdt, name, &tmp, sizeof(tmp));
 }
 static inline int fdt_property_cell(void *fdt, const char *name, uint32_t val)
 {
@@ -1154,8 +1154,8 @@ int fdt_setprop(void *fdt, int nodeoffset, const char *name,
 static inline int fdt_setprop_u32(void *fdt, int nodeoffset, const char *name,
 				  uint32_t val)
 {
-	val = cpu_to_fdt32(val);
-	return fdt_setprop(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt32_t tmp = cpu_to_fdt32(val);
+	return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
@@ -1189,8 +1189,8 @@ static inline int fdt_setprop_u32(void *fdt, int nodeoffset, const char *name,
 static inline int fdt_setprop_u64(void *fdt, int nodeoffset, const char *name,
 				  uint64_t val)
 {
-	val = cpu_to_fdt64(val);
-	return fdt_setprop(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt64_t tmp = cpu_to_fdt64(val);
+	return fdt_setprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
@@ -1296,8 +1296,8 @@ int fdt_appendprop(void *fdt, int nodeoffset, const char *name,
 static inline int fdt_appendprop_u32(void *fdt, int nodeoffset,
 				     const char *name, uint32_t val)
 {
-	val = cpu_to_fdt32(val);
-	return fdt_appendprop(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt32_t tmp = cpu_to_fdt32(val);
+	return fdt_appendprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
@@ -1331,8 +1331,8 @@ static inline int fdt_appendprop_u32(void *fdt, int nodeoffset,
 static inline int fdt_appendprop_u64(void *fdt, int nodeoffset,
 				     const char *name, uint64_t val)
 {
-	val = cpu_to_fdt64(val);
-	return fdt_appendprop(fdt, nodeoffset, name, &val, sizeof(val));
+	fdt64_t tmp = cpu_to_fdt64(val);
+	return fdt_appendprop(fdt, nodeoffset, name, &tmp, sizeof(tmp));
 }
 
 /**
-- 
1.8.0

Re: [PATCH v3 3/4] dtc/libfdt: introduce fdt types for annotation by endian checkers

[David Gibson]

On Wed, Nov 14, 2012 at 06:59:58PM -0600, Kim Phillips wrote:
> Projects such as linux and u-boot run sparse on libfdt.  libfdt
> contains the notion of endianness via usage of endian conversion
> functions such as fdt32_to_cpu.  As such, in order to pass endian
> checks, libfdt has to annotate its fdt variables such that sparse
> can warn when mixing bitwise and regular integers.  This patch adds
> these new fdtXX_t types and, ifdef __CHECKER__ (a symbol sparse
> defines), includes the bitwise annotation.
> 
> Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> ---
> v2:
> adds bitwise awareness: determine host endianness manually, and
> annotate swabs with __force in fdtXX_to_cpu and cpu_to_fdtXX
> conversion functions (the inline endian condition checks are
> optimized out at compile time).  This allows us to be able to check
> libfdt bitwise conversions with sparse by building dtc with make
> CC=cgcc. v2 also moves fdt32 definitions from fdt.h to libfdt_env.h
> and changes fdt32 definitions to use __bitwise instead of __be32. No
> separate _FDT_SPARSE was introduced because there's no need: using
> __CHECKER__ directly is valid because it only occurs once, and in
> libfdt_env.h.
> In addition, the libfdt sparse fixes have been moved to a subsequent
> patch.
> 
> v3:  address comments from jdl:
> o single set of fdt typedefs, since __bitwise is not defined if !CHECKER
> o re-work EXTRACT_BYTE to take 'x' as a parameter, not a global
> o SWAB function macros converted to take lowercase 'x' as a parameter,
>   not a global
> 
>  libfdt/fdt.h        | 42 ++++++++++++++++----------------
>  libfdt/libfdt_env.h | 69 +++++++++++++++++++++++++++++++++++++++++------------
>  2 files changed, 75 insertions(+), 36 deletions(-)
> 
> diff --git a/libfdt/fdt.h b/libfdt/fdt.h
> index 48ccfd9..45dd134 100644
> --- a/libfdt/fdt.h
> +++ b/libfdt/fdt.h
[snip]
> diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h
> index 213d7fb..f0d97b9 100644
> --- a/libfdt/libfdt_env.h
> +++ b/libfdt/libfdt_env.h
> @@ -5,25 +5,64 @@
>  #include <stdint.h>
>  #include <string.h>
>  
> -#define EXTRACT_BYTE(n)	((unsigned long long)((uint8_t *)&x)[n])
> -static inline uint16_t fdt16_to_cpu(uint16_t x)
> -{
> -	return (EXTRACT_BYTE(0) << 8) | EXTRACT_BYTE(1);
> -}
> -#define cpu_to_fdt16(x) fdt16_to_cpu(x)
> +#ifdef __CHECKER__
> +#define __force __attribute__((force))
> +#define __bitwise __attribute__((bitwise))
> +#else
> +#define __force
> +#define __bitwise
> +#endif
> +
> +typedef uint16_t __bitwise fdt16_t;
> +typedef uint32_t __bitwise fdt32_t;
> +typedef uint64_t __bitwise fdt64_t;

I agree with Jon that the approach above is better than the earlier one.

> +#define EXTRACT_BYTE(x, n) ((unsigned long long)((uint8_t *)&x)[n])
> +#define __SWAB16(x) ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
> +#define __SWAB32(x) ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
> +		     (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
> +#define __SWAB64(x) ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
> +		     (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
> +		     (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
> +		     (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))

This is not right, or at least very misleading.  "swab" usually refers
to an unconditional byteswap.  But the macros above are nops on a
big-endian machine.

> -static inline uint32_t fdt32_to_cpu(uint32_t x)
> -{
> -	return (EXTRACT_BYTE(0) << 24) | (EXTRACT_BYTE(1) << 16) | (EXTRACT_BYTE(2) << 8) | EXTRACT_BYTE(3);
> +/*
> + * determine host endianness:
> + * *__first_byte is 0x11 on big endian systems
> + * *__first_byte is 0x44 on little endian systems
> + */
> +static const uint32_t __native = 0x11223344u;
> +static const uint8_t *__first_byte = (const uint8_t *)&__native;
> +
> +#define DEF_FDT_TO_CPU(bits) \
> +static inline uint##bits##_t fdt##bits##_to_cpu(fdt##bits##_t x) \
> +{ \
> +	if (*__first_byte == 0x11) \
> +		return (__force uint##bits##_t)x; \
> +	else \
> +		return (__force uint##bits##_t)__SWAB##bits(x); \
>  }
> -#define cpu_to_fdt32(x) fdt32_to_cpu(x)
> +DEF_FDT_TO_CPU(16)
> +DEF_FDT_TO_CPU(32)
> +DEF_FDT_TO_CPU(64)

In fact, I really don't see why you're rewriting the byteswapper
functions as part of this patch.  The existing versions aren't very
nice, but if you want to rewrite those, please do it in a separate
patch.

-- 
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

Re: [PATCH v3 3/4] dtc/libfdt: introduce fdt types for annotation by endian checkers

[Kim Phillips]

On Thu, 15 Nov 2012 15:43:40 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Nov 14, 2012 at 06:59:58PM -0600, Kim Phillips wrote:
> > +#define EXTRACT_BYTE(x, n) ((unsigned long long)((uint8_t *)&x)[n])
> > +#define __SWAB16(x) ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
> > +#define __SWAB32(x) ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
> > +		     (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
> > +#define __SWAB64(x) ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
> > +		     (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
> > +		     (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
> > +		     (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
> 
> This is not right, or at least very misleading.  "swab" usually refers
> to an unconditional byteswap.  But the macros above are nops on a
> big-endian machine.

but they don't get called on a big endian system.  This is the name
linux uses.  If you want them renamed, please suggest names - I
can't read your mind.

> > -static inline uint32_t fdt32_to_cpu(uint32_t x)
> > -{
> > -	return (EXTRACT_BYTE(0) << 24) | (EXTRACT_BYTE(1) << 16) | (EXTRACT_BYTE(2) << 8) | EXTRACT_BYTE(3);
> > +/*
> > + * determine host endianness:
> > + * *__first_byte is 0x11 on big endian systems
> > + * *__first_byte is 0x44 on little endian systems
> > + */
> > +static const uint32_t __native = 0x11223344u;
> > +static const uint8_t *__first_byte = (const uint8_t *)&__native;
> > +
> > +#define DEF_FDT_TO_CPU(bits) \
> > +static inline uint##bits##_t fdt##bits##_to_cpu(fdt##bits##_t x) \
> > +{ \
> > +	if (*__first_byte == 0x11) \
> > +		return (__force uint##bits##_t)x; \
> > +	else \
> > +		return (__force uint##bits##_t)__SWAB##bits(x); \
> >  }
> > -#define cpu_to_fdt32(x) fdt32_to_cpu(x)
> > +DEF_FDT_TO_CPU(16)
> > +DEF_FDT_TO_CPU(32)
> > +DEF_FDT_TO_CPU(64)
> 
> In fact, I really don't see why you're rewriting the byteswapper
> functions as part of this patch.  The existing versions aren't very
> nice, but if you want to rewrite those, please do it in a separate
> patch.

This patchseries is about silencing sparse warnings in linux,
u-boot, and libfdt.  Sparse is intelligent in that if you mismatch a
native type of value 0 to a bitwise restricted type, it won't call a
warning.  The existing short-circuiting of the byteswapper
functions, i.e., defining cpu_to_fdt32(x) to fdt32_to_cpu(x) and
vice versa doesn't allow for correct attribution propagation.
Therefore I chose to allow sparse to see the actual conversion.  If
you have any other ideas on how to silence sparse in libfdt, let me
know.

Kim

Re: [PATCH v3 3/4] dtc/libfdt: introduce fdt types for annotation by endian checkers

[David Gibson]

On Wed, Nov 14, 2012 at 11:12:04PM -0600, Kim Phillips wrote:
> On Thu, 15 Nov 2012 15:43:40 +1100
> David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> 
> > On Wed, Nov 14, 2012 at 06:59:58PM -0600, Kim Phillips wrote:
> > > +#define EXTRACT_BYTE(x, n) ((unsigned long long)((uint8_t *)&x)[n])
> > > +#define __SWAB16(x) ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
> > > +#define __SWAB32(x) ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
> > > +		     (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
> > > +#define __SWAB64(x) ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
> > > +		     (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
> > > +		     (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
> > > +		     (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
> > 
> > This is not right, or at least very misleading.  "swab" usually refers
> > to an unconditional byteswap.  But the macros above are nops on a
> > big-endian machine.
> 
> but they don't get called on a big endian system.

Yes, which means the nop-on-bigendian is double-implemented which is
also ugly.

> This is the name
> linux uses.

I'm not sure exactly which *swab* functions in Linux you're referring
to, but I'm pretty sure most of those are unconditional swaps.

>  If you want them renamed, please suggest names - I
> can't read your mind.

Well, FDT_TO_CPU would do.

> > > -static inline uint32_t fdt32_to_cpu(uint32_t x)
> > > -{
> > > -	return (EXTRACT_BYTE(0) << 24) | (EXTRACT_BYTE(1) << 16) | (EXTRACT_BYTE(2) << 8) | EXTRACT_BYTE(3);
> > > +/*
> > > + * determine host endianness:
> > > + * *__first_byte is 0x11 on big endian systems
> > > + * *__first_byte is 0x44 on little endian systems
> > > + */
> > > +static const uint32_t __native = 0x11223344u;
> > > +static const uint8_t *__first_byte = (const uint8_t *)&__native;
> > > +
> > > +#define DEF_FDT_TO_CPU(bits) \
> > > +static inline uint##bits##_t fdt##bits##_to_cpu(fdt##bits##_t x) \
> > > +{ \
> > > +	if (*__first_byte == 0x11) \
> > > +		return (__force uint##bits##_t)x; \
> > > +	else \
> > > +		return (__force uint##bits##_t)__SWAB##bits(x); \
> > >  }
> > > -#define cpu_to_fdt32(x) fdt32_to_cpu(x)
> > > +DEF_FDT_TO_CPU(16)
> > > +DEF_FDT_TO_CPU(32)
> > > +DEF_FDT_TO_CPU(64)
> > 
> > In fact, I really don't see why you're rewriting the byteswapper
> > functions as part of this patch.  The existing versions aren't very
> > nice, but if you want to rewrite those, please do it in a separate
> > patch.
> 
> This patchseries is about silencing sparse warnings in linux,
> u-boot, and libfdt.  Sparse is intelligent in that if you mismatch a
> native type of value 0 to a bitwise restricted type, it won't call a
> warning.  The existing short-circuiting of the byteswapper
> functions, i.e., defining cpu_to_fdt32(x) to fdt32_to_cpu(x) and
> vice versa doesn't allow for correct attribution propagation.

Ah, right, yes that will have to go.  You've also added the explicit
endianness test, though, which is a redundant change.

> Therefore I chose to allow sparse to see the actual conversion.  If
> you have any other ideas on how to silence sparse in libfdt, let me
> know.

Hrm.  So you could either rename the macros, or just duplicate the
code in the to versions of the functions.

-- 
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

Re: [PATCH v4 3/4] dtc/libfdt: introduce fdt types for annotation by endian checkers

[Kim Phillips]

Projects such as linux and u-boot run sparse on libfdt.  libfdt
contains the notion of endianness via usage of endian conversion
functions such as fdt32_to_cpu.  As such, in order to pass endian
checks, libfdt has to annotate its fdt variables such that sparse
can warn when mixing bitwise and regular integers.  This patch adds
these new fdtXX_t types and, ifdef __CHECKER__ (a symbol sparse
defines), includes the bitwise annotation.

Signed-off-by: Kim Phillips <kim.phillips@freescale.com>

---
v2:
adds bitwise awareness: determine host endianness manually, and
annotate swabs with __force in fdtXX_to_cpu and cpu_to_fdtXX
conversion functions (the inline endian condition checks are
optimized out at compile time).  This allows us to be able to check
libfdt bitwise conversions with sparse by building dtc with make
CC=cgcc. v2 also moves fdt32 definitions from fdt.h to libfdt_env.h
and changes fdt32 definitions to use __bitwise instead of __be32. No
separate _FDT_SPARSE was introduced because there's no need: using
__CHECKER__ directly is valid because it only occurs once, and in
libfdt_env.h.
In addition, the libfdt sparse fixes have been moved to a subsequent
patch.

v3:  address comments from jdl:
o single set of fdt typedefs, since __bitwise is not defined if !CHECKER
o re-work EXTRACT_BYTE to take 'x' as a parameter, not a global
o SWAB function macros converted to take lowercase 'x' as a parameter,
  not a global

v4:  address comments from David Gibson
o rename byte swap routines CPU_TO_FDTXX
o properly define fdt<->cpu conversion routines
  to use CPU_TO_FDTXX macros with correct checker attributions.

 libfdt/fdt.h        | 42 +++++++++++++++++++++---------------------
 libfdt/libfdt_env.h | 53 ++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 63 insertions(+), 32 deletions(-)

diff --git a/libfdt/fdt.h b/libfdt/fdt.h
index 48ccfd9..45dd134 100644
--- a/libfdt/fdt.h
+++ b/libfdt/fdt.h
@@ -4,45 +4,45 @@
 #ifndef __ASSEMBLY__
 
 struct fdt_header {
-	uint32_t magic;			 /* magic word FDT_MAGIC */
-	uint32_t totalsize;		 /* total size of DT block */
-	uint32_t off_dt_struct;		 /* offset to structure */
-	uint32_t off_dt_strings;	 /* offset to strings */
-	uint32_t off_mem_rsvmap;	 /* offset to memory reserve map */
-	uint32_t version;		 /* format version */
-	uint32_t last_comp_version;	 /* last compatible version */
+	fdt32_t magic;			 /* magic word FDT_MAGIC */
+	fdt32_t totalsize;		 /* total size of DT block */
+	fdt32_t off_dt_struct;		 /* offset to structure */
+	fdt32_t off_dt_strings;		 /* offset to strings */
+	fdt32_t off_mem_rsvmap;		 /* offset to memory reserve map */
+	fdt32_t version;		 /* format version */
+	fdt32_t last_comp_version;	 /* last compatible version */
 
 	/* version 2 fields below */
-	uint32_t boot_cpuid_phys;	 /* Which physical CPU id we're
+	fdt32_t boot_cpuid_phys;	 /* Which physical CPU id we're
 					    booting on */
 	/* version 3 fields below */
-	uint32_t size_dt_strings;	 /* size of the strings block */
+	fdt32_t size_dt_strings;	 /* size of the strings block */
 
 	/* version 17 fields below */
-	uint32_t size_dt_struct;	 /* size of the structure block */
+	fdt32_t size_dt_struct;		 /* size of the structure block */
 };
 
 struct fdt_reserve_entry {
-	uint64_t address;
-	uint64_t size;
+	fdt64_t address;
+	fdt64_t size;
 };
 
 struct fdt_node_header {
-	uint32_t tag;
+	fdt32_t tag;
 	char name[0];
 };
 
 struct fdt_property {
-	uint32_t tag;
-	uint32_t len;
-	uint32_t nameoff;
+	fdt32_t tag;
+	fdt32_t len;
+	fdt32_t nameoff;
 	char data[0];
 };
 
 #endif /* !__ASSEMBLY */
 
 #define FDT_MAGIC	0xd00dfeed	/* 4: version, 4: total size */
-#define FDT_TAGSIZE	sizeof(uint32_t)
+#define FDT_TAGSIZE	sizeof(fdt32_t)
 
 #define FDT_BEGIN_NODE	0x1		/* Start node: full name */
 #define FDT_END_NODE	0x2		/* End node */
@@ -51,10 +51,10 @@ struct fdt_property {
 #define FDT_NOP		0x4		/* nop */
 #define FDT_END		0x9
 
-#define FDT_V1_SIZE	(7*sizeof(uint32_t))
-#define FDT_V2_SIZE	(FDT_V1_SIZE + sizeof(uint32_t))
-#define FDT_V3_SIZE	(FDT_V2_SIZE + sizeof(uint32_t))
+#define FDT_V1_SIZE	(7*sizeof(fdt32_t))
+#define FDT_V2_SIZE	(FDT_V1_SIZE + sizeof(fdt32_t))
+#define FDT_V3_SIZE	(FDT_V2_SIZE + sizeof(fdt32_t))
 #define FDT_V16_SIZE	FDT_V3_SIZE
-#define FDT_V17_SIZE	(FDT_V16_SIZE + sizeof(uint32_t))
+#define FDT_V17_SIZE	(FDT_V16_SIZE + sizeof(fdt32_t))
 
 #endif /* _FDT_H */
diff --git a/libfdt/libfdt_env.h b/libfdt/libfdt_env.h
index 213d7fb..956b4ae 100644
--- a/libfdt/libfdt_env.h
+++ b/libfdt/libfdt_env.h
@@ -5,25 +5,56 @@
 #include <stdint.h>
 #include <string.h>
 
-#define EXTRACT_BYTE(n)	((unsigned long long)((uint8_t *)&x)[n])
-static inline uint16_t fdt16_to_cpu(uint16_t x)
+#ifdef __CHECKER__
+#define __force __attribute__((force))
+#define __bitwise __attribute__((bitwise))
+#else
+#define __force
+#define __bitwise
+#endif
+
+typedef uint16_t __bitwise fdt16_t;
+typedef uint32_t __bitwise fdt32_t;
+typedef uint64_t __bitwise fdt64_t;
+
+#define EXTRACT_BYTE(x, n)	((unsigned long long)((uint8_t *)&x)[n])
+#define CPU_TO_FDT16(x) ((EXTRACT_BYTE(x, 0) << 8) | EXTRACT_BYTE(x, 1))
+#define CPU_TO_FDT32(x) ((EXTRACT_BYTE(x, 0) << 24) | (EXTRACT_BYTE(x, 1) << 16) | \
+			 (EXTRACT_BYTE(x, 2) << 8) | EXTRACT_BYTE(x, 3))
+#define CPU_TO_FDT64(x) ((EXTRACT_BYTE(x, 0) << 56) | (EXTRACT_BYTE(x, 1) << 48) | \
+			 (EXTRACT_BYTE(x, 2) << 40) | (EXTRACT_BYTE(x, 3) << 32) | \
+			 (EXTRACT_BYTE(x, 4) << 24) | (EXTRACT_BYTE(x, 5) << 16) | \
+			 (EXTRACT_BYTE(x, 6) << 8) | EXTRACT_BYTE(x, 7))
+
+static inline uint16_t fdt16_to_cpu(fdt16_t x)
+{
+	return (__force uint16_t)CPU_TO_FDT16(x);
+}
+static inline fdt16_t cpu_to_fdt16(uint16_t x)
 {
-	return (EXTRACT_BYTE(0) << 8) | EXTRACT_BYTE(1);
+	return (__force fdt16_t)CPU_TO_FDT16(x);
 }
-#define cpu_to_fdt16(x) fdt16_to_cpu(x)
 
-static inline uint32_t fdt32_to_cpu(uint32_t x)
+static inline uint32_t fdt32_to_cpu(fdt32_t x)
 {
-	return (EXTRACT_BYTE(0) << 24) | (EXTRACT_BYTE(1) << 16) | (EXTRACT_BYTE(2) << 8) | EXTRACT_BYTE(3);
+	return (__force uint32_t)CPU_TO_FDT32(x);
+}
+static inline fdt32_t cpu_to_fdt32(uint32_t x)
+{
+	return (__force fdt32_t)CPU_TO_FDT32(x);
 }
-#define cpu_to_fdt32(x) fdt32_to_cpu(x)
 
-static inline uint64_t fdt64_to_cpu(uint64_t x)
+static inline uint64_t fdt64_to_cpu(fdt64_t x)
+{
+	return (__force uint64_t)CPU_TO_FDT64(x);
+}
+static inline fdt64_t cpu_to_fdt64(uint64_t x)
 {
-	return (EXTRACT_BYTE(0) << 56) | (EXTRACT_BYTE(1) << 48) | (EXTRACT_BYTE(2) << 40) | (EXTRACT_BYTE(3) << 32)
-		| (EXTRACT_BYTE(4) << 24) | (EXTRACT_BYTE(5) << 16) | (EXTRACT_BYTE(6) << 8) | EXTRACT_BYTE(7);
+	return (__force fdt64_t)CPU_TO_FDT64(x);
 }
-#define cpu_to_fdt64(x) fdt64_to_cpu(x)
+#undef CPU_TO_FDT64
+#undef CPU_TO_FDT32
+#undef CPU_TO_FDT16
 #undef EXTRACT_BYTE
 
 #endif /* _LIBFDT_ENV_H */
-- 
1.8.0.1

Re: [PATCH v4 3/4] dtc/libfdt: introduce fdt types for annotation by endian checkers

[David Gibson]

On Wed, Nov 28, 2012 at 05:33:01PM -0600, Kim Phillips wrote:
> Projects such as linux and u-boot run sparse on libfdt.  libfdt
> contains the notion of endianness via usage of endian conversion
> functions such as fdt32_to_cpu.  As such, in order to pass endian
> checks, libfdt has to annotate its fdt variables such that sparse
> can warn when mixing bitwise and regular integers.  This patch adds
> these new fdtXX_t types and, ifdef __CHECKER__ (a symbol sparse
> defines), includes the bitwise annotation.
> 
> Signed-off-by: Kim Phillips <kim.phillips@freescale.com>

Much better, thanks

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
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

Re: [PATCH v4 3/4] dtc/libfdt: introduce fdt types for annotation by endian checkers

[Jon Loeliger]

> Projects such as linux and u-boot run sparse on libfdt.  libfdt
> contains the notion of endianness via usage of endian conversion
> functions such as fdt32_to_cpu.  As such, in order to pass endian
> checks, libfdt has to annotate its fdt variables such that sparse
> can warn when mixing bitwise and regular integers.  This patch adds
> these new fdtXX_t types and, ifdef __CHECKER__ (a symbol sparse
> defines), includes the bitwise annotation.
> 
> Signed-off-by: Kim Phillips <kim.phillips-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

Applied.

Thanks,
jdl