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[4]: *** [Makefile:573: procmap.lo] Error 1 gmake[4]: *** 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: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253839
Created attachment 222847 [details] AdHoc patch 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. https://reviews.freebsd.org/D29050 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: Author: cy Date: Thu Mar 4 16:14:45 UTC 2021 New revision: 567321 URL: https://svnweb.freebsd.org/changeset/ports/567321 Log: 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 sys/inode.h. PR: 253928, 253870 Submitted by: nyan _ myuji.xyz Reported by: cy, madpilot Reviewed by: cy MFH: 2021Q1 Differential Revision: https://reviews.freebsd.org/D29050 Changes: head/devel/libgtop/files/patch-sysdeps_freebsd_procmap.c
As the reporter, since this is now fixed, should I close this or should this bug stay open for some other reason?
Fixed.