After updating to a recent head I noticed libgtop failing, with error at the end of this message.
The mentioned commit add the bool return type to the inmem function.
I noticed this bool type is defined in sys/types.h, but only if _KERNEL is defined.
So the first time it is included _KERNEL needs to be defined.
I'm trying to get a patch to that effect, but until now I've always been missing some previous inclusion.
I'll followup with it as soon as I have a working patch.
In file included from procmap.c:54:
In file included from /usr/include/ufs/ufs/inode.h:50:
/usr/include/sys/buf.h:569:1: error: unknown type name 'bool'
bool inmem(struct vnode *, daddr_t);
procmap.c:101:25: warning: unused variable 'um' [-Wunused-variable]
struct ufsmount um;
procmap.c:405:16: warning: cast from 'gchar *' (aka 'char *') to 'glibtop_map_entry *' (aka 'struct _glibtop_map_entry *') increases required alignment from 1 to 8 [-Wcast-align]
return (glibtop_map_entry*) g_array_free(maps, FALSE);
2 warnings and 1 error generated.
gmake: *** [Makefile:573: procmap.lo] Error 1
gmake: *** Waiting for unfinished jobs....
Forgot to mention, I submitted this information as a bug against base, which resulted as invalid, I'm referencing it here:
Created attachment 222847 [details]
AdHoc patch to allow build on main.
Applicable on /usr/ports.
Tested only on main at commit 172f2fc11cc560bbd3d3b29260a8ae21df6a541b.
So possibly break other branches (stable/*, releng/*, ...).
Created attachment 222851 [details]
Patch using sys/types.h
I'm using a slightly different patch.
Actually that bool type should be the one expected by kernel own sources, which is defined in types.h
Hoisting it's inclusion and forcing _KERNEL defined warrants the correct includes.
I'm not able to judge which one is the best approach.
Obviously I'm biased towards my own patch :)
(In reply to Guido Falsi from comment #3)
Personally, whichever fix is OK.
Indeed, I first thought of your fix.
But reading bug 253839, I switched to use stdbool.h.
Just FYI, I switched my thought because...
*sys/types.h introduces much more things, possibly conflict future update.
*I feel FreeBSD project is going to decrease name space pollution.
*stdbool.h is NOT a kernel header, while headers in sys directory are
kernel headers. See HIER (7).
Unfortunately, kernel INTERNAL headers and (if any or not) kernel EXPOSED
headers are mixed up.
Considering what Konstantin said in bug 253839 stricter, in userspace apps,
including <sys/*> would better avoided whenever possible.
(In reply to Tomoaki AOKI from comment #4)
Basic problem is that the source itself is including and using system includes and already polluting things with _KERNEL. I grabbed sys/types.h because it is what buf.h seems to expect.
Anyway in this case what we are missing is only the bool definition, and I suspect that the function returning bool and causing the error is not even used in this code.
This means we could get away with a typedef bool to int and everything would work.
I really have no idea what is the "correct" solution. Ideal solution would be fixing this code to not use system includes at all.
I tested the non-kernel header approach; runtime seems fine with sysutils/gnome-system-monitor. I'm inclined to go with this approach due to bug 253839.
Maybe slightly unrelated, but since there have not been any releases since 2.40.0, other operating system distros have tracked commits after/on top of the release tag. Some of those commits include FreeBSD-specific/guarded code.
I just figured out this ticket after submitting a review.
Mine approach is slightly different, that to defer the
inclusion of ufs/ufs/inode.h, therefore, the kernel specific
symbols in sys/buf.h will not be included at all.
(In reply to nyan from comment #7)
A bit more of the justification to defer the inclusion after
undef _KERNEL is that, #define _KERNEL is userspace is really
hacky and simply feels wrong, since code in _KERNEL are really
meant to be used for and by kernel only, which means there are
no guarantee on api/kpi stability.
I personally feels a more correct approach will be to avoid
_KERNEL as much as possible to avoid further changes in kernel
specific code affects the port.
A commit references this bug:
Date: Thu Mar 4 16:14:45 UTC 2021
New revision: 567321
devel/libgtop: Fix build on 14-CURRENT post 2bfd8992c7c7
sys/inode.h now implicitly includes sys/buf.h which which itself has
dependencies requiring repositioning of inode.h.
This patch also references the inode itself instead of relying on the VTOI
macro thereby removing the need to define the _KERNEL macro when including
PR: 253928, 253870
Submitted by: nyan _ myuji.xyz
Reported by: cy, madpilot
Reviewed by: cy
Differential Revision: https://reviews.freebsd.org/D29050
As the reporter, since this is now fixed, should I close this or should this bug stay open for some other reason?