Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 27 Jun 2019 23:17:59 +0300
From:      Andriy Gapon <avg@FreeBSD.org>
To:        Bruce Evans <brde@optusnet.com.au>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r349459 - head/sys/sys
Message-ID:  <bdfad10f-de6d-0a83-288d-c7c79e8469f0@FreeBSD.org>
In-Reply-To: <20190628032225.T2863@besplex.bde.org>
References:  <201906271507.x5RF775Q070616@repo.freebsd.org> <20190628012059.D2091@besplex.bde.org> <dec79f80-3b00-15ad-cd63-6e6ecb83e176@FreeBSD.org> <20190628032225.T2863@besplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 27/06/2019 20:37, Bruce Evans wrote:
> On Thu, 27 Jun 2019, Andriy Gapon wrote:
> 
>> On 27/06/2019 18:47, Bruce Evans wrote:
>>> On Thu, 27 Jun 2019, Andriy Gapon wrote:
>>>
>>>> Log:
>>>>  upgrade the warning printf-s in bus accessors to KASSERT-s
>>>>
>>>>  After this change sys/bus.h includes sys/systm.h.
>>>
>>> This is further namespace pollution.  sys/systm.h is a prerequiste for
>>> all kernel headers except sys/param.h and the headers that that includes,
>>> since any kernel header (except sys/param.hm etc.) may have inlines which
>>> use features in systm.h, e.g., KASSERT() or an inline function.
>>
>> what do you think about amending style(9) to require that if sys/systm.h is to
>> be included, then it must be included before any other header except for
>> sys/param.h (and possibly sys/cdefs.h) ?
> 
> It is not a style matter.

I know... but style(9) documents sys/param.h for similar reasons.

> sys/systm.h is just a prerequisite for almost
> all kernel code.  Perhaps this can be enforced using #ifdef magic even
> for headers that don't currently use it.  If it is to be included nested,
> then sys/param.h is the place to include it, but I don't like that since
> it is even less related to parameters than sys/param.h, and this would be
> a regression from minor depollution of sys/param.h.
> 
> sys/systm.h is also kernel-only, and as you found, including it nested
> tends to break applications, so other headers that have the bug of
> including it tend to have _KERNEL ifdefs to avoid including it in
> applications.  This is so fundamental that it doesn't have a "No
> user-serviceable parts inside" ifdef.

I think that there is a trivial fix to my commit: moving the sys/systm.h include
to the _KERNEL section of sys/bus.h.  That's where its definitions are actually
used.
But I agree that there should be a better approach.


-- 
Andriy Gapon



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?bdfad10f-de6d-0a83-288d-c7c79e8469f0>