On 05.02.2024 16:55, Julien Grall wrote: > On 05/02/2024 15:14, Andrew Cooper wrote: >> Use pragmas to able the warning in this file only. All supported versions of >> Clang understand this, while older GCCs simply ignore it. >> >> bitmap_find_free_region() is the only function which isn't sign-convert >> clean. This highlights a latent bug in that it can't return successfully for >> a bitmap larger than 2G. >> >> Add an extra check, and explicit cast to silence the warning. >> >> Signed-off-by: Andrew Cooper <[email protected]> >> --- >> CC: George Dunlap <[email protected]> >> CC: Jan Beulich <[email protected]> >> CC: Stefano Stabellini <[email protected]> >> CC: Wei Liu <[email protected]> >> CC: Julien Grall <[email protected]> >> >> Slightly RFC. This is our first use of pragmas like this. > > The only other approach I can think of is specifying the CFLAGS per file > like Linux did. I don't know if our build system supports that though.
It does, see e.g. # Allows usercopy.c to include itself $(obj)/usercopy.o: CFLAGS-y += -iquote . in arch/x86/Makefile. > AFAICT, the only advantage would be to avoid duplicating the pragmas. So > this is not a strong preference. My other concern there are old gcc versions we still support. I haven't checked (yet) when support for these pragma-s was introduced; I only know they haven't been there forever. However, ... >> --- a/xen/common/bitmap.c >> +++ b/xen/common/bitmap.c >> @@ -14,6 +14,9 @@ >> #include <xen/lib.h> >> #include <asm/byteorder.h> >> >> +#pragma GCC diagnostic warning "-Wsign-conversion" >> +#pragma clang diagnostic warning "-Wsign-conversion" >> + > > OOI, any reason why wasn't added at the right at the top of the file? ... this may be relevant: Inline functions may have an issue with being processed with the warning enabled. Otoh it may also be a problem if the warning isn't enabled for them. Jan
