Hi André, On Thu, 17 Sep 2020 at 04:27, André Przywara <andre.przywara@xxxxxxx> wrote: > > On 17/01/2020 09:23, David Gibson wrote: > > Hi, > > > On Thu, Jan 16, 2020 at 08:58:12PM +1300, Simon Glass wrote: > >> Hi David, > >> > >> On Thu, 16 Jan 2020 at 19:50, David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote: > >>> > >>> On Sun, Jan 12, 2020 at 11:52:08AM -0700, Simon Glass wrote: > >>>> These warnings appear when building U-Boot on x86 and some other targets. > >>>> Correct them by adding casts. > >>>> > >>>> Example: > >>>> > >>>> scripts/dtc/libfdt/fdt.c: In function ‘fdt_offset_ptr’: > >>>> scripts/dtc/libfdt/fdt.c:137:18: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare] > >>>> if ((absoffset < offset) > >>>> > >>>> Signed-off-by: Simon Glass <sjg@xxxxxxxxxxxx> > >>> > >>> Hmm. So squashing warnings is certainly a good thing in general. > >>> > >>> Unfortunately, I'm really uncomfortable with most of these changes. > >>> In a number of cases they are outright wrong. In most of the others, > >>> the code was already correct. I dislike adding casts to suppress > >>> spurious warnings on correct code because that can end up hiding real > >>> problems which might be introduced by future changes. > >>> > >>> Case by case details below. > >> > >> Thanks for the review. I agree this is all really horrible, > >> particularly in light of the fact that it doesn't fix bugs and perhaps > >> introduces some. > >> > >> This was just a quick patch to silence the warnings. If we do make > >> fixes here we should really make sure that there are test cases to > >> trigger each check. I suspect we have some but not all. > > > > Yeah, adding some safety test cases for egregiously bad input like > > negative buffer sizes is probably a good idea. > > > >> What do you think we should do? The warnings are going to be very > >> annoying for people. I could perhaps split the patch up and work > >> through things one by one. > > > > Yeah, we want to find some way to remove the warnings, and I think > > splitting up and working piece by piece will be necessary. > > Has anyone done anything on that front? > If not, I would take a deep breath and try to tackle this one by one. I > was grudgingly ignoring this in U-Boot so far, but it popped up in > Trusted Firmware now as well, so I have a business reason (TM). No, I never got back to it. It would be great if you could take it on! Regards, Simon
