Re: [RFC] Push BKL into chrdev opens

[Dave Hansen <haveblue@us.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 379 bytes --]

Kevin O'Connor wrote:
> The returnout macro is incredibly ugly.  It is also broken.  (The "goto
> out" is on the same level as the if and is executed regardless of the if
> condition.)

You are very, very right.  Ugh.  I made this during a 10-second moment 
of weakness when I wondered if we should have a macro to do this.

Cleanup attached.
-- 
Dave Hansen
haveblue@us.ibm.com

[-- Attachment #2: cdev-bkl-2.5.30+bk-7-istallionfix.patch --]
[-- Type: text/plain, Size: 4474 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.545   -> 1.546  
#	drivers/char/istallion.c	1.13    -> 1.14   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/08/02	haveblue@nighthawk.sr71.net	1.546
# fix ugliness
# --------------------------------------------
#
diff -Nru a/drivers/char/istallion.c b/drivers/char/istallion.c
--- a/drivers/char/istallion.c	Fri Aug  2 11:42:09 2002
+++ b/drivers/char/istallion.c	Fri Aug  2 11:42:09 2002
@@ -1022,8 +1022,6 @@
 
 /*****************************************************************************/
 
-#define returnout(x) ret=x;goto out;
-
 static int stli_open(struct tty_struct *tty, struct file *filp)
 {
 	stlibrd_t	*brdp;
@@ -1038,23 +1036,34 @@
 
 	minordev = minor(tty->device);
 	brdnr = MINOR2BRD(minordev);
-	if (brdnr >= stli_nrbrds)
-		returnout(-ENODEV);
+	if (brdnr >= stli_nrbrds) {
+		ret = -ENODEV; 
+		goto out;
+	}
 	brdp = stli_brds[brdnr];
-	if (brdp == (stlibrd_t *) NULL)
-		returnout(-ENODEV);
-	if ((brdp->state & BST_STARTED) == 0)
-		returnout(-ENODEV);
+	if (brdp == (stlibrd_t *) NULL) {
+		ret = -ENODEV;
+		goto out;
+	}
+	if ((brdp->state & BST_STARTED) == 0) {
+		ret = -ENODEV;
+		goto out;
+	}
 	portnr = MINOR2PORT(minordev);
-	if ((portnr < 0) || (portnr > brdp->nrports))
-		returnout(-ENODEV);
+	if ((portnr < 0) || (portnr > brdp->nrports)) {
+		ret = -ENODEV;
+		goto out;
+	}
 
 	portp = brdp->ports[portnr];
-	if (portp == (stliport_t *) NULL)
-		returnout(-ENODEV);
-	if (portp->devnr < 1)
-		returnout(-ENODEV);
-
+	if (portp == (stliport_t *) NULL) {
+		ret = -ENODEV;
+		goto out;
+	}
+	if (portp->devnr < 1) {
+		ret =-ENODEV;
+		goto out;
+	}
 	MOD_INC_USE_COUNT;
 
 /*
@@ -1065,9 +1074,12 @@
  */
 	if (portp->flags & ASYNC_CLOSING) {
 		interruptible_sleep_on(&portp->close_wait);
-		if (portp->flags & ASYNC_HUP_NOTIFY)
-			returnout(-EAGAIN);
-		returnout(-ERESTARTSYS);
+		if (portp->flags & ASYNC_HUP_NOTIFY) {
+			ret = -EAGAIN;
+			goto out;
+		}
+		ret = -ERESTARTSYS;
+		goto out;
 	}
 
 /*
@@ -1081,8 +1093,10 @@
 	portp->refcount++;
 
 	while (test_bit(ST_INITIALIZING, &portp->state)) {
-		if (signal_pending(current))
-			returnout(-ERESTARTSYS);
+		if (signal_pending(current)) {
+			ret = -ERESTARTSYS;
+			goto out;
+		}
 		interruptible_sleep_on(&portp->raw_wait);
 	}
 
@@ -1094,8 +1108,10 @@
 		}
 		clear_bit(ST_INITIALIZING, &portp->state);
 		wake_up_interruptible(&portp->raw_wait);
-		if (rc < 0)
-			returnout(rc);
+		if (rc < 0) {
+			ret = rc;
+			goto out;
+		}
 	}
 
 /*
@@ -1106,9 +1122,12 @@
  */
 	if (portp->flags & ASYNC_CLOSING) {
 		interruptible_sleep_on(&portp->close_wait);
-		if (portp->flags & ASYNC_HUP_NOTIFY)
-			returnout(-EAGAIN);
-		returnout(-ERESTARTSYS);
+		if (portp->flags & ASYNC_HUP_NOTIFY) {
+			ret = -EAGAIN;
+			goto out;
+		}
+		ret = -ERESTARTSYS;
+		goto out;
 	}
 
 /*
@@ -1117,24 +1136,34 @@
  *	then also we might have to wait for carrier.
  */
 	if (tty->driver.subtype == STL_DRVTYPCALLOUT) {
-		if (portp->flags & ASYNC_NORMAL_ACTIVE)
-			returnout(-EBUSY);
+		if (portp->flags & ASYNC_NORMAL_ACTIVE) {
+			ret = -EBUSY;
+			goto out;
+		}
 		if (portp->flags & ASYNC_CALLOUT_ACTIVE) {
 			if ((portp->flags & ASYNC_SESSION_LOCKOUT) &&
-			    (portp->session != current->session))
-				returnout(-EBUSY);
+			    (portp->session != current->session)) {
+				ret = -EBUSY;
+				goto out;
+			}
 			if ((portp->flags & ASYNC_PGRP_LOCKOUT) &&
-			    (portp->pgrp != current->pgrp))
-				returnout(-EBUSY);
+			    (portp->pgrp != current->pgrp)) {
+				ret = -EBUSY);
+				goto out;
+			}
 		}
 		portp->flags |= ASYNC_CALLOUT_ACTIVE;
 	} else {
 		if (filp->f_flags & O_NONBLOCK) {
-			if (portp->flags & ASYNC_CALLOUT_ACTIVE)
-				returnout(-EBUSY);
+			if (portp->flags & ASYNC_CALLOUT_ACTIVE) {
+				ret = -EBUSY;
+				goto out;
+			}
 		} else {
-			if ((rc = stli_waitcarrier(brdp, portp, filp)) != 0)
-				returnout(rc);
+			if ((rc = stli_waitcarrier(brdp, portp, filp)) != 0) {
+				ret = rc;
+				goto out;
+			}
 		}
 		portp->flags |= ASYNC_NORMAL_ACTIVE;
 	}
@@ -1151,7 +1180,7 @@
 	portp->pgrp = current->pgrp;
 out:
 	unlock_kernel();
-	return 0;
+	return ret;
 }
 
 /*****************************************************************************/