Date: Fri, 28 Sep 2001 19:39:33 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Warner Losh <imp@harmony.village.org> Cc: <arch@FreeBSD.org> Subject: Re: Proposed change: d_thread_t for driver portability between 4.x and 5.x Message-ID: <20010928192532.M52718-100000@delplex.bde.org> In-Reply-To: <200109272330.f8RNUn776995@harmony.village.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 27 Sep 2001, Warner Losh wrote:
> Right now I have to do some really ugly things in the pcic driver to
> make my driver portable between 4.x and 5.x. Surprising, the only
> ugly part is that stable has struct proc and current has struct thread
> for many parameters. Most drivers just shuffle these poitners around
> to various routines and don't need to look under the covers.
>
> As such, having a d_thread_t typedef would make it easier to write
> drivers that can be shared. Instead of saying struct proc *p, you'd
> say d_thread_t *p in its place. This will make it work for both
> -stable and -current without further kludges.
This breaks 2 style rules :-). From style(9):
Avoid using typedefs for structure types. Such typedefs make it impossi-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ble for applications to use pointers to such a structure opaquely, which
is both possible and beneficial when using an ordinary struct tag. When
convention requires a typedef, make its name match the struct tag. Avoid
^^^^^
typedefs ending in ``_t'', except as specified in Standard C or by POSIX.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
These are wollman's rules, not KNF. I strongly agree with them, but we have
too many of these personal rules which not everyone agreed to.
I can see the use of d_thread_t^Hs if it is truly opaque. Declaring it
only in <sys/conf.h> or even including <sys/conf.h> breaks its opaqueness.
Unfortunately, the other rule-breaking declarations like d_open_t require
including the full <sys/conf.h> in device drivers. It's another bug
that dev_t is not either truly opaque or declared as "struct foo *".
> Here's the diffs for -current:
>
> Index: conf.h
> ===================================================================
> RCS file: /home/imp/FreeBSD/CVS/src/sys/sys/conf.h,v
> retrieving revision 1.133
> diff -u -r1.133 conf.h
> --- conf.h 2001/09/12 08:38:05 1.133
> +++ conf.h 2001/09/27 23:19:49
> @@ -114,17 +114,18 @@
> struct uio;
> struct knote;
>
> -typedef int d_open_t __P((dev_t dev, int oflags, int devtype, struct thread *td));
> -typedef int d_close_t __P((dev_t dev, int fflag, int devtype, struct thread *td));
> +typedef struct thread d_thread_t;
> +typedef int d_open_t __P((dev_t dev, int oflags, int devtype, d_thread_t *td));
> +typedef int d_close_t __P((dev_t dev, int fflag, int devtype, d_thread_t *td));
The type need/should not be opaque to the implementation, so there should
only be 1 1-line change to this file (to add a typedef).
Bruce
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-arch" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010928192532.M52718-100000>