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
