[PATCH] libfdt: Fix signedness comparison warnings

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] libfdt: Fix signedness comparison warnings

Andre Przywara-3
This is a combination of upstream libfdt commits to fix warnings about
comparing signed and unsigned integers:
==========
scripts/dtc/libfdt/fdt.c: In function ‘fdt_offset_ptr’:
scripts/dtc/libfdt/fdt.c:137:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
   if ((absoffset < offset)
...
==========

For a detailed description of the fixes, see the dtc repo:
https://git.kernel.org/pub/scm/utils/dtc/dtc.git/log/?id=73e0f143b73d808

For this patch the commits between 73e0f143b73d8088 and ca19c3db2bf62000
have been combined and adjusted for the slight differences in U-Boot's
libfdt code base.

Signed-off-by: Andre Przywara <[hidden email]>
---
 scripts/dtc/libfdt/fdt.c          | 15 +++++++++++----
 scripts/dtc/libfdt/fdt_overlay.c  |  3 ++-
 scripts/dtc/libfdt/fdt_ro.c       | 20 +++++++++++---------
 scripts/dtc/libfdt/fdt_strerror.c |  4 ++--
 scripts/dtc/libfdt/fdt_sw.c       | 27 +++++++++++++++------------
 scripts/dtc/libfdt/fdt_wip.c      |  2 +-
 6 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/scripts/dtc/libfdt/fdt.c b/scripts/dtc/libfdt/fdt.c
index 8e4cce3b9be..28f4e1a5f15 100644
--- a/scripts/dtc/libfdt/fdt.c
+++ b/scripts/dtc/libfdt/fdt.c
@@ -131,16 +131,20 @@ int fdt_check_header(const void *fdt)
 
 const void *fdt_offset_ptr(const void *fdt, int offset, unsigned int len)
 {
- unsigned absoffset = offset + fdt_off_dt_struct(fdt);
+ unsigned int uoffset = offset;
+ unsigned int absoffset = offset + fdt_off_dt_struct(fdt);
+
+ if (offset < 0)
+ return NULL;
 
  if (fdt_chk_basic())
- if ((absoffset < offset)
+ if ((absoffset < uoffset)
     || ((absoffset + len) < absoffset)
     || (absoffset + len) > fdt_totalsize(fdt))
  return NULL;
 
  if (!fdt_chk_version() || fdt_version(fdt) >= 0x11)
- if (((offset + len) < offset)
+ if (((uoffset + len) < uoffset)
     || ((offset + len) > fdt_size_dt_struct(fdt)))
  return NULL;
 
@@ -302,9 +306,12 @@ const char *fdt_find_string_(const char *strtab, int tabsize, const char *s)
 
 int fdt_move(const void *fdt, void *buf, int bufsize)
 {
+ if (fdt_chk_basic() && bufsize < 0)
+ return -FDT_ERR_NOSPACE;
+
  FDT_RO_PROBE(fdt);
 
- if (fdt_totalsize(fdt) > bufsize)
+ if (fdt_totalsize(fdt) > (unsigned int)bufsize)
  return -FDT_ERR_NOSPACE;
 
  memmove(buf, fdt, fdt_totalsize(fdt));
diff --git a/scripts/dtc/libfdt/fdt_overlay.c b/scripts/dtc/libfdt/fdt_overlay.c
index bd75e3dd786..7a65c35af6f 100644
--- a/scripts/dtc/libfdt/fdt_overlay.c
+++ b/scripts/dtc/libfdt/fdt_overlay.c
@@ -241,6 +241,7 @@ static int overlay_update_local_node_references(void *fdto,
 
  if (fixup_len % sizeof(uint32_t))
  return -FDT_ERR_BADOVERLAY;
+ fixup_len /= sizeof(uint32_t);
 
  tree_val = fdt_getprop(fdto, tree_node, name, &tree_len);
  if (!tree_val) {
@@ -250,7 +251,7 @@ static int overlay_update_local_node_references(void *fdto,
  return tree_len;
  }
 
- for (i = 0; i < (fixup_len / sizeof(uint32_t)); i++) {
+ for (i = 0; i < fixup_len; i++) {
  fdt32_t adj_val;
  uint32_t poffset;
 
diff --git a/scripts/dtc/libfdt/fdt_ro.c b/scripts/dtc/libfdt/fdt_ro.c
index d9d52e0d56e..d984bab036b 100644
--- a/scripts/dtc/libfdt/fdt_ro.c
+++ b/scripts/dtc/libfdt/fdt_ro.c
@@ -53,7 +53,7 @@ const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
 
  err = -FDT_ERR_BADOFFSET;
  absoffset = stroffset + fdt_off_dt_strings(fdt);
- if (absoffset >= totalsize)
+ if (absoffset >= (unsigned)totalsize)
  goto fail;
  len = totalsize - absoffset;
 
@@ -61,17 +61,19 @@ const char *fdt_get_string(const void *fdt, int stroffset, int *lenp)
  if (stroffset < 0)
  goto fail;
  if (!fdt_chk_version() || fdt_version(fdt) >= 17) {
- if (stroffset >= fdt_size_dt_strings(fdt))
+ if ((unsigned)stroffset >= fdt_size_dt_strings(fdt))
  goto fail;
  if ((fdt_size_dt_strings(fdt) - stroffset) < len)
  len = fdt_size_dt_strings(fdt) - stroffset;
  }
  } else if (fdt_magic(fdt) == FDT_SW_MAGIC) {
- if ((stroffset >= 0)
-    || (stroffset < -fdt_size_dt_strings(fdt)))
+ unsigned int sw_stroffset = -stroffset;
+
+ if ((stroffset >= 0) ||
+    (sw_stroffset > fdt_size_dt_strings(fdt)))
  goto fail;
- if ((-stroffset) < len)
- len = -stroffset;
+ if ((sw_stroffset) < len)
+ len = sw_stroffset;
  } else {
  err = -FDT_ERR_INTERNAL;
  goto fail;
@@ -157,8 +159,8 @@ int fdt_generate_phandle(const void *fdt, uint32_t *phandle)
 
 static const struct fdt_reserve_entry *fdt_mem_rsv(const void *fdt, int n)
 {
- int offset = n * sizeof(struct fdt_reserve_entry);
- int absoffset = fdt_off_mem_rsvmap(fdt) + offset;
+ unsigned int offset = n * sizeof(struct fdt_reserve_entry);
+ unsigned int absoffset = fdt_off_mem_rsvmap(fdt) + offset;
 
  if (fdt_chk_extra()) {
  if (absoffset < fdt_off_mem_rsvmap(fdt))
@@ -679,7 +681,7 @@ int fdt_node_offset_by_phandle(const void *fdt, uint32_t phandle)
 {
  int offset;
 
- if ((phandle == 0) || (phandle == -1))
+ if ((phandle == 0) || (phandle == ~0U))
  return -FDT_ERR_BADPHANDLE;
 
  FDT_RO_PROBE(fdt);
diff --git a/scripts/dtc/libfdt/fdt_strerror.c b/scripts/dtc/libfdt/fdt_strerror.c
index 768db66eada..b4356931b06 100644
--- a/scripts/dtc/libfdt/fdt_strerror.c
+++ b/scripts/dtc/libfdt/fdt_strerror.c
@@ -40,7 +40,7 @@ static struct fdt_errtabent fdt_errtable[] = {
  FDT_ERRTABENT(FDT_ERR_NOPHANDLES),
  FDT_ERRTABENT(FDT_ERR_BADFLAGS),
 };
-#define FDT_ERRTABSIZE (sizeof(fdt_errtable) / sizeof(fdt_errtable[0]))
+#define FDT_ERRTABSIZE ((int)(sizeof(fdt_errtable) / sizeof(fdt_errtable[0])))
 
 const char *fdt_strerror(int errval)
 {
@@ -48,7 +48,7 @@ const char *fdt_strerror(int errval)
  return "<valid offset/length>";
  else if (errval == 0)
  return "<no error>";
- else if (errval > -FDT_ERRTABSIZE) {
+ else if (-errval < FDT_ERRTABSIZE) {
  const char *s = fdt_errtable[-errval].str;
 
  if (s)
diff --git a/scripts/dtc/libfdt/fdt_sw.c b/scripts/dtc/libfdt/fdt_sw.c
index a8c924675a6..d9e67fa2a61 100644
--- a/scripts/dtc/libfdt/fdt_sw.c
+++ b/scripts/dtc/libfdt/fdt_sw.c
@@ -96,8 +96,8 @@ static inline uint32_t sw_flags(void *fdt)
 
 static void *fdt_grab_space_(void *fdt, size_t len)
 {
- int offset = fdt_size_dt_struct(fdt);
- int spaceleft;
+ unsigned int offset = fdt_size_dt_struct(fdt);
+ unsigned int spaceleft;
 
  spaceleft = fdt_totalsize(fdt) - fdt_off_dt_struct(fdt)
  - fdt_size_dt_strings(fdt);
@@ -111,8 +111,8 @@ static void *fdt_grab_space_(void *fdt, size_t len)
 
 int fdt_create_with_flags(void *buf, int bufsize, uint32_t flags)
 {
- const size_t hdrsize = FDT_ALIGN(sizeof(struct fdt_header),
- sizeof(struct fdt_reserve_entry));
+ const int hdrsize = FDT_ALIGN(sizeof(struct fdt_header),
+      sizeof(struct fdt_reserve_entry));
  void *fdt = buf;
 
  if (bufsize < hdrsize)
@@ -155,13 +155,16 @@ int fdt_resize(void *fdt, void *buf, int bufsize)
 
  FDT_SW_PROBE(fdt);
 
+ if (bufsize < 0)
+ return -FDT_ERR_NOSPACE;
+
  headsize = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
  tailsize = fdt_size_dt_strings(fdt);
 
  if (fdt_chk_extra() && (headsize + tailsize) > fdt_totalsize(fdt))
  return -FDT_ERR_INTERNAL;
 
- if ((headsize + tailsize) > bufsize)
+ if ((headsize + tailsize) > (unsigned)bufsize)
  return -FDT_ERR_NOSPACE;
 
  oldtail = (char *)fdt + fdt_totalsize(fdt) - tailsize;
@@ -249,18 +252,18 @@ int fdt_end_node(void *fdt)
 static int fdt_add_string_(void *fdt, const char *s)
 {
  char *strtab = (char *)fdt + fdt_totalsize(fdt);
- int strtabsize = fdt_size_dt_strings(fdt);
- int len = strlen(s) + 1;
- int struct_top, offset;
+ unsigned int strtabsize = fdt_size_dt_strings(fdt);
+ unsigned int len = strlen(s) + 1;
+ unsigned int struct_top, offset;
 
- offset = -strtabsize - len;
+ offset = strtabsize + len;
  struct_top = fdt_off_dt_struct(fdt) + fdt_size_dt_struct(fdt);
- if (fdt_totalsize(fdt) + offset < struct_top)
+ if (fdt_totalsize(fdt) - offset < struct_top)
  return 0; /* no more room :( */
 
- memcpy(strtab + offset, s, len);
+ memcpy(strtab - offset, s, len);
  fdt_set_size_dt_strings(fdt, strtabsize + len);
- return offset;
+ return -offset;
 }
 
 /* Must only be used to roll back in case of error */
diff --git a/scripts/dtc/libfdt/fdt_wip.c b/scripts/dtc/libfdt/fdt_wip.c
index f64139e0b3d..c2d7566a67d 100644
--- a/scripts/dtc/libfdt/fdt_wip.c
+++ b/scripts/dtc/libfdt/fdt_wip.c
@@ -23,7 +23,7 @@ int fdt_setprop_inplace_namelen_partial(void *fdt, int nodeoffset,
  if (!propval)
  return proplen;
 
- if (proplen < (len + idx))
+ if ((unsigned)proplen < (len + idx))
  return -FDT_ERR_NOSPACE;
 
  memcpy((char *)propval + idx, val, len);
--
2.17.5

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libfdt: Fix signedness comparison warnings

Tom Rini-4
On Fri, Oct 16, 2020 at 03:42:50PM +0100, Andre Przywara wrote:

> This is a combination of upstream libfdt commits to fix warnings about
> comparing signed and unsigned integers:
> ==========
> scripts/dtc/libfdt/fdt.c: In function ‘fdt_offset_ptr’:
> scripts/dtc/libfdt/fdt.c:137:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
>    if ((absoffset < offset)
> ...
> ==========
>
> For a detailed description of the fixes, see the dtc repo:
> https://git.kernel.org/pub/scm/utils/dtc/dtc.git/log/?id=73e0f143b73d808
>
> For this patch the commits between 73e0f143b73d8088 and ca19c3db2bf62000
> have been combined and adjusted for the slight differences in U-Boot's
> libfdt code base.
>
> Signed-off-by: Andre Przywara <[hidden email]>
So, the scripts that the Linux kernel uses to re-sync with dtc also work
for U-Boot.  Has the kernel re-synced yet?  If so, can we just re-sync
with that same commit again?  That's typically how we do this.  Thanks!

--
Tom

signature.asc (673 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] libfdt: Fix signedness comparison warnings

Tom Rini-4
On Fri, Oct 16, 2020 at 10:57:19AM -0400, Tom Rini wrote:

> On Fri, Oct 16, 2020 at 03:42:50PM +0100, Andre Przywara wrote:
>
> > This is a combination of upstream libfdt commits to fix warnings about
> > comparing signed and unsigned integers:
> > ==========
> > scripts/dtc/libfdt/fdt.c: In function ‘fdt_offset_ptr’:
> > scripts/dtc/libfdt/fdt.c:137:18: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
> >    if ((absoffset < offset)
> > ...
> > ==========
> >
> > For a detailed description of the fixes, see the dtc repo:
> > https://git.kernel.org/pub/scm/utils/dtc/dtc.git/log/?id=73e0f143b73d808
> >
> > For this patch the commits between 73e0f143b73d8088 and ca19c3db2bf62000
> > have been combined and adjusted for the slight differences in U-Boot's
> > libfdt code base.
> >
> > Signed-off-by: Andre Przywara <[hidden email]>
>
> So, the scripts that the Linux kernel uses to re-sync with dtc also work
> for U-Boot.  Has the kernel re-synced yet?  If so, can we just re-sync
> with that same commit again?  That's typically how we do this.  Thanks!
I see that it has.  So I'll give a re-sync a try and see what comes up.
Thanks!

--
Tom

signature.asc (673 bytes) Download Attachment