Re: Fix nagging make_dev() warnings

[Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx>]

:An inspection of sys/kern/tty_cons.c shows that the reported replacement 
:"sc" -> "intercept" is indeed intentional. It just means that the dev_ops 
:has been substituted as desired. Then, however, requiring
:
:[*]	(si->si_ops == ops && si->si_udev == udev)
:
:in hashdev() looks inappropriate, as this prevents the intercepted device 
:from being successfully looked up from the hash table. Strictly speaking, 
:dev_ops_get(12,255) returns the ops of the original device, and this is 
:used by udev2dev() for a subsequent hashdev() call. As a consequence one 
:has si_ops != ops and [*] is not matched. Once the interception is 
:established, the next attempt to open consolectl thus results in the 
:creation of a new device whose si_ops is the original one. This effects 
:a complete bypassing of the intercepted device.
:
:I can't figure out why the condition "si->si_ops == ops" in [*] had been 
:added. For testing purposes I dropped this to see what goes wrong, but 
:everything still looks fine. Anyway, in view if the new intercept code it 
:appears that here is a point which should be reconsidered.
:
:Regards,
:Frank Josellis

    Ok, it took a few readings to decipher your posting, but I understand
    what you are getting at.

    Basically the interception is only modifying the device and is not
    modifying the link structure, so when udev2dev() calls dev_ops_get(),
    dev_ops_get() returns the original ops structure instead of the
    intercepted ops structure and this results in hashdev() being
    called with the wrong ops.

    The second part of this is more uncertain.  Do we even *want* udev2dev()
    to create new devices with the override ops instead of the original
    ops if one does not already exist?  I don't think we do, because
    the console override code would have no visibility into the newly
    created devices using its override ops vector.   In fact, I don't think
    this case can occur anyway since the console device must already exist
    in order to be overriden.

    So this leaves us with just the first bug... the interception is causing
    udev2dev() to call hashdev() with the original ops vector and thus 
    create a new device with the original ops instead of finding the device
    with the overridden ops.

    I hate doing hacks, but I think in this case I'd rather solve the
    problem quickly and get it off my radar screen.  Here's a patch, I
    am committing it right now.

					-Matt
					Matthew Dillon 
					<dillon@xxxxxxxxxxxxx>

Index: kern_conf.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_conf.c,v
retrieving revision 1.15
diff -u -r1.15 kern_conf.c
--- kern_conf.c	10 Sep 2006 01:26:39 -0000	1.15
+++ kern_conf.c	26 Sep 2006 17:57:16 -0000
@@ -114,7 +114,7 @@
  */
 static
 cdev_t
-hashdev(struct dev_ops *ops, int x, int y)
+hashdev(struct dev_ops *ops, int x, int y, int allow_override)
 {
 	struct cdev *si;
 	udev_t	udev;
@@ -124,8 +124,10 @@
 	udev = makeudev(x, y);
 	hash = udev % DEVT_HASH;
 	LIST_FOREACH(si, &dev_hash[hash], si_hash) {
-		if (si->si_ops == ops && si->si_udev == udev)
+		if (si->si_udev == udev &&
+		    (allow_override || si->si_ops == ops)) {
 			return (si);
+		}
 	}
 	if (stashed >= DEVT_STASH) {
 		MALLOC(si, struct cdev *, sizeof(*si), M_DEVT,
@@ -185,7 +187,7 @@
 	ops = dev_ops_get(umajor(x), uminor(x));
 	if (ops == NULL)
 		return(NOCDEV);
-	dev = hashdev(ops, umajor(x), uminor(x));
+	dev = hashdev(ops, umajor(x), uminor(x), TRUE);
 	return(dev);
 }
 
@@ -245,7 +247,7 @@
 	 * compile the cdevsw and install the device
 	 */
 	compile_dev_ops(ops);
-	dev = hashdev(ops, ops->head.maj, minor);
+	dev = hashdev(ops, ops->head.maj, minor, FALSE);
 
 	/*
 	 * Set additional fields (XXX DEVFS interface goes here)
@@ -267,7 +269,7 @@
 {
 	cdev_t dev;
 
-	dev = hashdev(ops, ops->head.maj, minor);
+	dev = hashdev(ops, ops->head.maj, minor, FALSE);
 	return(dev);
 }
 
@@ -280,7 +282,7 @@
 {
 	cdev_t	dev;
 
-	dev = hashdev(odev->si_ops, umajor(odev->si_udev), minor);
+	dev = hashdev(odev->si_ops, umajor(odev->si_udev), minor, FALSE);
 
 	/*
 	 * Copy cred requirements and name info XXX DEVFS.