please test sndstat lor patch

Mathew Kanner mat at cnd.mcgill.ca
Thu Nov 27 10:29:28 PST 2003


Hello Gang,
	I wrote this to cleanup /dev/sndstat a little and fix a LOR.
Then I couldn't remember if there is a LOR or not.  Could someone
please remind me?  If we do, could someone please test this patch.

	Thanks,
	--Mat

-- 
	The beaver, which has come to represent Canada as the eagle
	does the United States and the lion Britain, is a
	flat-tailed, slow-witted, toothy rodent known to bite off
	it's own testicles or to stand under its own falling trees.
			- June Callwood
-------------- next part --------------
Index: sndstat.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/sound/pcm/sndstat.c,v
retrieving revision 1.14
diff -u -r1.14 sndstat.c
--- sndstat.c	7 Sep 2003 16:28:03 -0000	1.14
+++ sndstat.c	27 Nov 2003 18:21:06 -0000
@@ -56,44 +56,41 @@
 	int type, unit;
 };
 
-#ifdef	USING_MUTEX
-static struct mtx sndstat_lock;
-#endif
-static struct sbuf sndstat_sbuf;
 static dev_t sndstat_dev = 0;
 static int sndstat_isopen = 0;
-static int sndstat_bufptr;
+
+struct sndstat {
+	void *rawbuf;
+	struct sbuf sbuf;
+	int bufptr;
+	int isopen;
+};
+
+static struct mtx sndstat_lock;
 static int sndstat_maxunit = -1;
 static int sndstat_files = 0;
 
 static SLIST_HEAD(, sndstat_entry) sndstat_devlist = SLIST_HEAD_INITIALIZER(none);
 
 static int sndstat_verbose = 1;
-#ifdef	USING_MUTEX
 TUNABLE_INT("hw.snd.verbose", &sndstat_verbose);
-#else
-TUNABLE_INT_DECL("hw.snd.verbose", 1, sndstat_verbose);
-#endif
 
 static int sndstat_prepare(struct sbuf *s);
 
 static int
 sysctl_hw_sndverbose(SYSCTL_HANDLER_ARGS)
 {
-	intrmask_t s;
 	int error, verbose;
 
 	verbose = sndstat_verbose;
 	error = sysctl_handle_int(oidp, &verbose, sizeof(verbose), req);
 	if (error == 0 && req->newptr != NULL) {
-		s = spltty();
 		mtx_lock(&sndstat_lock);
 		if (verbose < 0 || verbose > 3)
 			error = EINVAL;
 		else
 			sndstat_verbose = verbose;
 		mtx_unlock(&sndstat_lock);
-		splx(s);
 	}
 	return error;
 }
@@ -103,32 +100,26 @@
 static int
 sndstat_open(dev_t i_dev, int flags, int mode, struct thread *td)
 {
-	intrmask_t s;
+	struct sndstat *s = i_dev->si_drv1;
 	int error;
 
-	s = spltty();
+	if (s == NULL)
+		return ENXIO;
+
 	mtx_lock(&sndstat_lock);
 	if (sndstat_isopen) {
 		mtx_unlock(&sndstat_lock);
-		splx(s);
 		return EBUSY;
 	}
-	sndstat_isopen = 1;
+	sndstat_isopen = s->isopen = 1;
 	mtx_unlock(&sndstat_lock);
-	splx(s);
-	if (sbuf_new(&sndstat_sbuf, NULL, 4096, 0) == NULL) {
-		error = ENXIO;
-		goto out;
-	}
-	sndstat_bufptr = 0;
-	error = (sndstat_prepare(&sndstat_sbuf) > 0) ? 0 : ENOMEM;
-out:
+	sbuf_clear(&s->sbuf);
+	s->bufptr = 0;
+	error = (sndstat_prepare(&s->sbuf) > 0) ? 0 : ENOMEM;
 	if (error) {
-		s = spltty();
 		mtx_lock(&sndstat_lock);
-		sndstat_isopen = 0;
+		sndstat_isopen = s->isopen = 0;
 		mtx_unlock(&sndstat_lock);
-		splx(s);
 	}
 	return (error);
 }
@@ -136,42 +127,40 @@
 static int
 sndstat_close(dev_t i_dev, int flags, int mode, struct thread *td)
 {
-	intrmask_t s;
+	struct sndstat *s = i_dev->si_drv1;
+	if (s == NULL)
+		return ENXIO;
 
-	s = spltty();
 	mtx_lock(&sndstat_lock);
 	if (!sndstat_isopen) {
 		mtx_unlock(&sndstat_lock);
-		splx(s);
 		return EBADF;
 	}
-	sbuf_delete(&sndstat_sbuf);
-	sndstat_isopen = 0;
+	sndstat_isopen = s->isopen = 0;
 
 	mtx_unlock(&sndstat_lock);
-	splx(s);
 	return 0;
 }
 
 static int
 sndstat_read(dev_t i_dev, struct uio *buf, int flag)
 {
-	intrmask_t s;
+	struct sndstat *s = i_dev->si_drv1;
 	int l, err;
 
-	s = spltty();
+	if (s == NULL)
+		return ENXIO;
+
 	mtx_lock(&sndstat_lock);
 	if (!sndstat_isopen) {
 		mtx_unlock(&sndstat_lock);
-		splx(s);
 		return EBADF;
 	}
-    	l = min(buf->uio_resid, sbuf_len(&sndstat_sbuf) - sndstat_bufptr);
-	err = (l > 0)? uiomove(sbuf_data(&sndstat_sbuf) + sndstat_bufptr, l, buf) : 0;
-	sndstat_bufptr += l;
+    	l = min(buf->uio_resid, sbuf_len(&s->sbuf) - s->bufptr);
+	err = (l > 0)? uiomove(sbuf_data(&s->sbuf) + s->bufptr, l, buf) : 0;
+	s->bufptr += l;
 
 	mtx_unlock(&sndstat_lock);
-	splx(s);
 	return err;
 }
 
@@ -193,7 +182,6 @@
 int
 sndstat_register(device_t dev, char *str, sndstat_handler handler)
 {
-	intrmask_t s;
 	struct sndstat_entry *ent;
 	const char *devtype;
 	int type, unit;
@@ -214,7 +202,7 @@
 		unit = -1;
 	}
 
-	ent = malloc(sizeof *ent, M_DEVBUF, M_ZERO | M_WAITOK);
+	ent = malloc(sizeof *ent, M_DEVBUF, M_ZERO | M_NOWAIT);
 	if (!ent)
 		return ENOSPC;
 
@@ -224,14 +212,12 @@
 	ent->unit = unit;
 	ent->handler = handler;
 
-	s = spltty();
 	mtx_lock(&sndstat_lock);
 	SLIST_INSERT_HEAD(&sndstat_devlist, ent, link);
 	if (type == SS_TYPE_MODULE)
 		sndstat_files++;
 	sndstat_maxunit = (unit > sndstat_maxunit)? unit : sndstat_maxunit;
 	mtx_unlock(&sndstat_lock);
-	splx(s);
 
 	return 0;
 }
@@ -245,23 +231,19 @@
 int
 sndstat_unregister(device_t dev)
 {
-	intrmask_t s;
 	struct sndstat_entry *ent;
 
-	s = spltty();
 	mtx_lock(&sndstat_lock);
 	SLIST_FOREACH(ent, &sndstat_devlist, link) {
 		if (ent->dev == dev) {
 			SLIST_REMOVE(&sndstat_devlist, ent, sndstat_entry, link);
 			mtx_unlock(&sndstat_lock);
-			splx(s);
 			free(ent, M_DEVBUF);
 
 			return 0;
 		}
 	}
 	mtx_unlock(&sndstat_lock);
-	splx(s);
 
 	return ENXIO;
 }
@@ -269,24 +251,20 @@
 int
 sndstat_unregisterfile(char *str)
 {
-	intrmask_t s;
 	struct sndstat_entry *ent;
 
-	s = spltty();
 	mtx_lock(&sndstat_lock);
 	SLIST_FOREACH(ent, &sndstat_devlist, link) {
 		if (ent->dev == NULL && ent->str == str) {
 			SLIST_REMOVE(&sndstat_devlist, ent, sndstat_entry, link);
 			sndstat_files--;
 			mtx_unlock(&sndstat_lock);
-			splx(s);
 			free(ent, M_DEVBUF);
 
 			return 0;
 		}
 	}
 	mtx_unlock(&sndstat_lock);
-	splx(s);
 
 	return ENXIO;
 }
@@ -338,32 +316,83 @@
 }
 
 static int
-sndstat_init(void)
-{
+sndstat_init(void) {
+	struct sndstat *s;
+	int ret;
+
 	mtx_init(&sndstat_lock, "sndstat", NULL, 0);
-	sndstat_dev = make_dev(&sndstat_cdevsw, SND_DEV_STATUS, UID_ROOT, GID_WHEEL, 0444, "sndstat");
 
-	return (sndstat_dev != 0)? 0 : ENXIO;
+	/*
+	 * Except for sndstat_lock, this is a generic device creation
+	 * suitable for multiple devices.  I don't know why you would
+	 * want that though.
+	 */
+	ret = ENOMEM;
+
+
+	s = malloc(sizeof(s), M_DEVBUF, M_ZERO|M_NOWAIT);
+	if (s == NULL)
+		goto err;
+
+	s->rawbuf = malloc(4096, M_TEMP, M_NOWAIT);
+	if (s->rawbuf == NULL)
+		goto err;
+
+	sbuf_new(&s->sbuf, s->rawbuf, 4096, SBUF_FIXEDLEN);
+
+	sndstat_dev = make_dev(&sndstat_cdevsw, SND_DEV_STATUS, UID_ROOT, GID_WHEEL, 0444, "sndstat");
+	if (sndstat_dev == NULL) {
+		return ENXIO;
+		goto err;
+	}
+	sndstat_dev->si_drv1 = s;
+
+	/*
+	 * Device created OK
+	 */
+	ret = 0;
+	goto ok;
+
+err:	if (s != NULL) {
+		if (s->rawbuf != NULL) {
+			free(s->rawbuf, M_TEMP);
+			sbuf_delete(&s->sbuf);
+		}
+		free(s, M_DEVBUF);
+	}
+ok:
+	return ret;
 }
 
 static int
-sndstat_uninit(void)
-{
-	intrmask_t s;
+sndstat_uninit(void) {
+
+	struct sndstat *s = sndstat_dev->si_drv1;
+
+	if (s == NULL)
+		return ENXIO;
+
+	/*
+	 * XXX: I'm puzzled at what the correct behaviour should be since
+	 * uninit might fail, do we destroy the lock anyway?  Since init/unit
+	 * is called from SYSINIT the return value is ignored and we can't
+	 * veto.  We aren't allowed to fail.
+	 * Use old semantics until this is figured out.
+	 */
 
-	s = spltty();
 	mtx_lock(&sndstat_lock);
 	if (sndstat_isopen) {
 		mtx_unlock(&sndstat_lock);
-		splx(s);
 		return EBUSY;
 	}
 
-	if (sndstat_dev)
-		destroy_dev(sndstat_dev);
+	sbuf_delete(&s->sbuf);
+	free(s->rawbuf, M_TEMP);
+	free(s, M_DEVBUF);
+
+	destroy_dev(sndstat_dev);
 	sndstat_dev = 0;
 
-	splx(s);
 	mtx_destroy(&sndstat_lock);
 	return 0;
 }


More information about the freebsd-current mailing list