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

[Kamil Rytarowski <n54%gmx.com@localhost>]

On 01.04.2020 16:17, Christos Zoulas wrote:
> 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
>

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.