NetBSD Problem Report #58582

From sender@gorgnet.net  Sun Aug 11 14:15:35 2024
Return-Path: <sender@gorgnet.net>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits)
	 client-signature RSA-PSS (2048 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 18EC31A9243
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 11 Aug 2024 14:15:35 +0000 (UTC)
Message-Id: <20240811141527.2233.qmail@gorgnet.net>
Date: 11 Aug 2024 14:15:27 -0000
From: gorg@gorgnet.net
Reply-To: gorg@gorgnet.net
To: gnats-bugs@NetBSD.org
Subject: [PATCH] Do not rely on outside inclusion in rasops.h and wsdisplay_vconsvar.h
X-Send-Pr-Version: 3.95

>Number:         58582
>Category:       kern
>Synopsis:       Do not rely on outside inclusion in rasops.h and wsdisplay_vconsvar.h
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sun Aug 11 14:20:00 +0000 2024
>Last-Modified:  Sun Aug 11 15:45:01 +0000 2024
>Originator:     gorg@gorgnet.net
>Release:        NetBSD 10.0
>Organization:
>Environment:
System: NetBSD 10.0 NetBSD 10.0 (GENERIC64) #0: Thu Mar 28 08:33:33 UTC 2024 mkrepro@mkrepro.NetBSD.org:/usr/src/sys/arch/evbarm/compile/GENERIC64 evbarm
Architecture: aarch64
Machine: evbarm
>Description:
In their current forms, rasops.h and wsdisplay_vconsvar.h require the inclusion
of either wsdisplayvar.h or wsdisplayvar.h and rasops.h, respectively, by the
source file that includes them. This is because they use symbols from these
other header files without including such header files themselves, contrary to
the NetBSD source code style guide:

http://cvsweb.netbsd.org/bsdweb.cgi/src/share/misc/style?rev=1.77&content-type=text/x-cvsweb-markup

   155	 * If a header file requires structures, defines, typedefs, etc. from
   156	 * another header file it should include that header file and not depend
   157	 * on the including file for that header including both.  If there are

These nonconformities require wsdisplay drivers to adhere to a certain relative
order of inclusion of these headers in order to compile, going against another
rule in the style guide pertaining to the order of includes. While this does
not seem like a very large problem to me, I hope it makes sense to submit this
PR as fixing nonconformant code.

I apologize if I missed or have misunderstood anything.

>How-To-Repeat:

>Fix:
If it is desirable for me to also correct the inclusion order within wsdisplay
drivers in this patch, please let me know and I will add changes to them.

Index: sys/dev/rasops/rasops.h
===================================================================
RCS file: /cvsroot/src/sys/dev/rasops/rasops.h,v
retrieving revision 1.50
diff -u -r1.50 rasops.h
--- sys/dev/rasops/rasops.h	24 Dec 2021 18:12:58 -0000	1.50
+++ sys/dev/rasops/rasops.h	10 Aug 2024 23:06:54 -0000
@@ -35,6 +35,7 @@
 #include <sys/param.h>

 #include <dev/wscons/wsconsio.h>
+#include <dev/wscons/wsdisplayvar.h>
 #include <dev/wsfont/wsfont.h>

 /* For rasops_info::ri_flg */
Index: sys/dev/wscons/wsdisplay_vconsvar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/wscons/wsdisplay_vconsvar.h,v
retrieving revision 1.34
diff -u -r1.34 wsdisplay_vconsvar.h
--- sys/dev/wscons/wsdisplay_vconsvar.h	14 Feb 2023 08:22:02 -0000	1.34
+++ sys/dev/wscons/wsdisplay_vconsvar.h	10 Aug 2024 23:06:54 -0000
@@ -36,6 +36,9 @@

 #include <sys/atomic.h>

+#include <dev/rasops/rasops.h>
+#include <dev/wscons/wsdisplayvar.h>
+
 struct vcons_data;

 struct vcons_screen {
Index: share/man/man9/rasops.9
===================================================================
RCS file: /cvsroot/src/share/man/man9/rasops.9,v
retrieving revision 1.19
diff -u -r1.19 rasops.9
--- share/man/man9/rasops.9	8 Aug 2019 00:20:54 -0000	1.19
+++ share/man/man9/rasops.9	10 Aug 2024 23:06:54 -0000
@@ -36,7 +36,6 @@
 .Nm rasops_reconfig
 .Nd raster display operations
 .Sh SYNOPSIS
-.In dev/wscons/wsdisplayvar.h
 .In dev/rasops/rasops.h
 .Ft int
 .Fn rasops_init "struct rasops_info *ri" "int wantrows" "int wantcols"

>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/58582: [PATCH] Do not rely on outside inclusion in rasops.h
 and wsdisplay_vconsvar.h
Date: Sun, 11 Aug 2024 17:13:16 +0200

 On Sun, Aug 11, 2024 at 02:20:00PM +0000, gorg@gorgnet.net wrote:
 > In their current forms, rasops.h and wsdisplay_vconsvar.h require the inclusion
 > of either wsdisplayvar.h or wsdisplayvar.h and rasops.h, respectively, by the
 > source file that includes them. This is because they use symbols from these
 > other header files without including such header files themselves, contrary to
 > the NetBSD source code style guide:

 IIRC conceptually rasops should be independent of wsdisplay (and used
 to be in the past), what symbols from there is it using?

 Martin

From: George Matsumura <gorg@gorgnet.net>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/58582: [PATCH] Do not rely on outside inclusion in rasops.h
 and wsdisplay_vconsvar.h
Date: Sun, 11 Aug 2024 09:42:07 -0600

 From wsdisplayvar.h specifically, it uses wsdisplay_emulops:

    151		struct	wsdisplay_emulops ri_ops;
    158		struct	wsdisplay_emulops ri_real_ops;

 From wsfont.h (present before this change), it uses wsdisplay_font:

     97		struct	wsdisplay_font *ri_font;
     98		struct	wsdisplay_font ri_optfont;

 I may have missed a few references, but as of now files including rasops.h do
 not seem to compile unless wsdisplayvar.h is included before it.

 Thank you for your time in looking at this.

 Regards,
     George

 On 8/11/24 09:15, gnats-admin@netbsd.org wrote:
 > The following reply was made to PR kern/58582; it has been noted by GNATS.
 > 
 > From: Martin Husemann <martin@duskware.de>
 > To: gnats-bugs@netbsd.org
 > Cc: 
 > Subject: Re: kern/58582: [PATCH] Do not rely on outside inclusion in rasops.h
 >  and wsdisplay_vconsvar.h
 > Date: Sun, 11 Aug 2024 17:13:16 +0200
 > 
 >  On Sun, Aug 11, 2024 at 02:20:00PM +0000, gorg@gorgnet.net wrote:
 >  > In their current forms, rasops.h and wsdisplay_vconsvar.h require the inclusion
 >  > of either wsdisplayvar.h or wsdisplayvar.h and rasops.h, respectively, by the
 >  > source file that includes them. This is because they use symbols from these
 >  > other header files without including such header files themselves, contrary to
 >  > the NetBSD source code style guide:
 >  
 >  IIRC conceptually rasops should be independent of wsdisplay (and used
 >  to be in the past), what symbols from there is it using?
 >  
 >  Martin
 >  

Home	PR Database Search