On 01.04.2020 16:17, Christos Zoulas wrote: > In article <pine.neb.4.64.2004010706530.4...@speedy.whooppee.com>, > Paul Goyette <p...@whooppee.com> wrote: >> On Wed, 1 Apr 2020, Kamil Rytarowski wrote: >> >>> On 01.04.2020 15:47, Robert Elz wrote: >>>> Date: Wed, 1 Apr 2020 11:45:53 +0000 >>>> From: "Kamil Rytarowski" <ka...@netbsd.org> >>>> Message-ID: <20200401114554.05167f...@cvs.netbsd.org> >>>> >>>> | Log Message: >>>> | Avoid comparison between signed and unsigned integer >>>> | >>>> | Cast PAGE_SIZE to size_t. >>>> >>>> This kind of pedantry is going way too far, PAGE_SIZE is a compile >>>> time constant (1 << PAGE_SHIFT) which is an int (and so signed, >>>> nominally) but one which is known to be positive. >>>> >>> >>> I got reports that certain ports no longer build due to: >>> >>> src/sys/modules/examples/fopsmapper/fopsmapper.c:118:11: error: >>> comparison between signed and unsigned integer expressions >>> [-Werror=sign-compare] >>> if (size != PAGE_SIZE) >>> ^~ >>> cc1: all warnings being treated as errors >> >> >> There's a lot of modules that fail for this with WARNS=5 when being >> built as loadable modules. >> >> That's why so many of the individual module Makefiles have explicit >> WARNS=4. (It seems that when modules are built-in as part of the >> kernel, they are by default built with WARNS=4.) > > Which we have been slowly fixing. I think in this case the sign-compare > warnings are annoying, but putting casts on each warning is cluttering > the code needlessly. Unfortunately the alternative (to make the PAGESIZE > constant unsigned is more dangerous -- unless of course you are willing to > examine all the places it is used and make sure that the semantics don't > change). Another way would be to make: > > #define PAGESIZE_U ((unsigned)PAGESIZE) > > and use that instead; eventually when all instances of PAGESIZE have been > converted to PAGESIZE_U it can be removed. But is it worth it? There are > perhaps better things to do. But anyway you want to proceed should be > discussed in tech-kern. Adding piecemeal casts everywhere does not make > the world a better place. > > christos >
Does it look better? http://netbsd.org/~kamil/patch-00244-fopsmapper-PAGE_SIZE.txt Perhaps we could switch PAGE_SIZE to unsigned.. but it is too much for now.
