Re: CVS commit: src/sys/modules/examples/fopsmapper

[christos%astron.com@localhost (Christos Zoulas)]

In article <Pine.NEB.4.64.2004010706530.4255%speedy.whooppee.com@localhost>,
Paul Goyette  <paul%whooppee.com@localhost> 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" <kamil%netbsd.org@localhost>
>>>     Message-ID:  <20200401114554.05167FB27%cvs.NetBSD.org@localhost>
>>>
>>>   | 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