[PATCH 00/14] libfdt: Fix signed/unsigned comparison warnings

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



When libfdt is compiled with -Wsign-compare or -Wextra, GCC emits quite
some warnings about the signedness of the operands not matching:
=================
libfdt/fdt.c:140:18: error: comparison between signed and unsigned integer expressions [-Werror=sign-compare]
   if ((absoffset < offset)
.....
=================

This does not occur under normal conditions in the dtc repo, but might
show up when libfdt is embedded in another project. There have been reports
from U-Boot and Trusted-Firmware-A.

The underlying issue is mostly due to C's promotion behaviour (ANSI C
section 6.1.3.8) when dealing with operands of different signedness
(but same size): Signed values get implictly casted to unsigned, which
is not typically what we want if they could have been negative.

The Internet(TM) suggests that blindly applying casts is probably doing
more harm than it helps, so this series tries to fix the underlying
issues properly.
In libfdt, some types are somewhat suboptimal ("int bufsize" comes to mind);
some signed types are due to them being returned along wih error values in
other functions (node offsets).
So these fixes here have been based on the following assumptions:
- We cannot change the prototype of exported functions.
- It's better to change types (for local variables) than to cast.
- If we have established that a signed value is not negative, we can safely
  cast it to an unsigned type.

I split up the fixes in small chunks, to make them easier to review.
The first four patches change types, the next six ones use casts after
we made sure the values are not negative.

This is only covering libfdt for now (which is what those other projects
care about). There are more issues with dtc, but they can be addressed
later.

Not sure if some of these checks should be gated by can_assume() calls.

Please have a look, happy to discuss the invididual cases.

Cheers,
Andre

Andre Przywara (13):
  libfdt: fdt_offset_ptr(): Fix comparison warnings
  libfdt: fdt_mem_rsv(): Fix comparison warnings
  libfdt: fdt_grab_space_(): Fix comparison warning
  libfdt: fdt_add_string_(): Fix comparison warning
  libfdt: fdt_move(): Fix comparison warnings
  libfdt: fdt_splice_(): Fix comparison warning
  libfdt: fdt_create_with_flags(): Fix comparison warning
  libfdt: fdt_resize(): Fix comparison warning
  libfdt: libfdt_wip: Fix comparison warning
  libfdt: overlay: Fix comparison warning
  libfdt: fdt_get_string(): Fix sequential write comparison warnings
  libfdt: fdt_node_offset_by_phandle(): Fix comparison warning
  libfdt: fdt_strerror(): Fix comparison warning

Simon Glass (1):
  libfdt: fdt_get_string(): Fix comparison warnings

 libfdt/fdt.c          | 15 +++++++++++----
 libfdt/fdt_overlay.c  |  3 ++-
 libfdt/fdt_ro.c       | 14 +++++++-------
 libfdt/fdt_rw.c       |  2 +-
 libfdt/fdt_strerror.c |  2 +-
 libfdt/fdt_sw.c       | 14 +++++++++-----
 libfdt/fdt_wip.c      |  4 ++--
 7 files changed, 33 insertions(+), 21 deletions(-)

-- 
2.17.5




[Index of Archives]     [Device Tree]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux