On Thu, Oct 01, 2020 at 05:46:24PM +0100, Andre Przywara wrote: > Those are the six remaining patches of the initial post to fix the > C comparison warnings. > I reworked the fixes according to David's comments, and took quite a > different approach for some of them. > Changelog below. > > The series is against https://github.com/dgibson/dtc/commits/main > ------------------------------------ > > 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. > > 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. > > Please have a look, happy to discuss the invididual cases. Thanks again for this work. I've applied all the remaining patches, although I have some comments for some followups I think would be good. At this point can we turn on -Wsign-compare by default? That sounds like a good idea to stop these problems creeping back in. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
