Some code cleanups done while working on locking. Reduces always-true

    tests, excessive indenation, convoluted handling of errors, or code
    duplication.

2 files changed, 106 insertions, 131 deletions

diff --git a/bsd/drm_bufs.h b/bsd/drm_bufs.h
index 5314f303..4c33763c 100644
--- a/bsd/drm_bufs.h
+++ b/bsd/drm_bufs.h
@@ -933,17 +933,17 @@ int DRM(mapbufs)( DRM_IOCTL_ARGS )
 	drm_device_dma_t *dma = dev->dma;
 	int retcode = 0;
 	const int zero = 0;
-	vm_offset_t virtual, address;
+	vm_offset_t address;
+	struct vmspace *vms;
 #ifdef __FreeBSD__
-#if __FreeBSD_version >= 500000
-	struct vmspace *vms = p->td_proc->p_vmspace;
-#else
-	struct vmspace *vms = p->p_vmspace;
-#endif
+	vm_ooffset_t foff;
+	vm_size_t size;
+	vm_offset_t vaddr;
 #endif /* __FreeBSD__ */
 #ifdef __NetBSD__
 	struct vnode *vn;
-	struct vmspace *vms = p->p_vmspace;
+	vm_size_t size;
+	vaddr_t vaddr;
 #endif /* __NetBSD__ */
 
 	drm_buf_map_t request;
@@ -962,98 +962,79 @@ int DRM(mapbufs)( DRM_IOCTL_ARGS )
 	DRM_COPY_FROM_USER_IOCTL( request, (drm_buf_map_t *)data, sizeof(request) );
 
 #ifdef __NetBSD__
-	if(!vfinddev(kdev, VCHR, &vn))
+	if (!vfinddev(kdev, VCHR, &vn))
 		return 0;	/* FIXME: Shouldn't this be EINVAL or something? */
 #endif /* __NetBSD__ */
 
-	if ( request.count >= dma->buf_count ) {
-		if ( (__HAVE_AGP && (dma->flags & _DRM_DMA_USE_AGP)) ||
-		     (__HAVE_SG && (dma->flags & _DRM_DMA_USE_SG)) ) {
-			drm_local_map_t *map = DRIVER_AGP_BUFFERS_MAP( dev );
+#if defined(__FreeBSD__) && __FreeBSD_version >= 500000
+	vms = p->td_proc->p_vmspace;
+#else
+	vms = p->p_vmspace;
+#endif
 
-			if ( !map ) {
-				retcode = EINVAL;
-				goto done;
-			}
+	if (request.count < dma->buf_count)
+		goto done;
+
+	if ((__HAVE_AGP && (dma->flags & _DRM_DMA_USE_AGP)) ||
+	    (__HAVE_SG && (dma->flags & _DRM_DMA_USE_SG))) {
+		drm_local_map_t *map = DRIVER_AGP_BUFFERS_MAP(dev);
+
+		if (map == NULL) {
+			retcode = EINVAL;
+			goto done;
+		}
+		size = round_page(map->size);
+		foff = map->offset;
+	} else {
+		size = round_page(dma->byte_count),
+		foff = 0;
+	}
 
 #ifdef __FreeBSD__
-			virtual = round_page((vm_offset_t)vms->vm_daddr + MAXDSIZ);
-			retcode = vm_mmap(&vms->vm_map,
-					  &virtual,
-					  round_page(map->size),
-					  PROT_READ|PROT_WRITE, VM_PROT_ALL,
-					  MAP_SHARED,
-					  SLIST_FIRST(&kdev->si_hlist),
-					  (unsigned long)map->offset );
-#elif defined(__NetBSD__)
-			virtual = round_page((vaddr_t)vms->vm_daddr + MAXDSIZ);
-			retcode = uvm_mmap(&vms->vm_map,
-					   (vaddr_t *)&virtual,
-					   round_page(map->size),
-					   UVM_PROT_READ | UVM_PROT_WRITE,
-					   UVM_PROT_ALL, MAP_SHARED,
-					   &vn->v_uobj, map->offset,
-					   p->p_rlimit[RLIMIT_MEMLOCK].rlim_cur);
-#endif /* __NetBSD__ */
-		} else {
-#ifdef __FreeBSD__
-			virtual = round_page((vm_offset_t)vms->vm_daddr + MAXDSIZ);
-			retcode = vm_mmap(&vms->vm_map,
-					  &virtual,
-					  round_page(dma->byte_count),
-					  PROT_READ|PROT_WRITE, VM_PROT_ALL,
-					  MAP_SHARED,
-					  SLIST_FIRST(&kdev->si_hlist),
-					  0);
+	vaddr = round_page((vm_offset_t)vms->vm_daddr + MAXDSIZ);
+	retcode = vm_mmap(&vms->vm_map, &vaddr, size, PROT_READ | PROT_WRITE,
+	    VM_PROT_ALL, MAP_SHARED, SLIST_FIRST(&kdev->si_hlist), foff );
 #elif defined(__NetBSD__)
-			virtual = round_page((vaddr_t)vms->vm_daddr + MAXDSIZ);
-			retcode = uvm_mmap(&vms->vm_map,
-					   (vaddr_t *)&virtual,
-					   round_page(dma->byte_count),
-					   UVM_PROT_READ | UVM_PROT_WRITE,
-					   UVM_PROT_ALL, MAP_SHARED,
-					   &vn->v_uobj, 0,
-					   p->p_rlimit[RLIMIT_MEMLOCK].rlim_cur);
+	vaddr = round_page((vaddr_t)vms->vm_daddr + MAXDSIZ);
+	retcode = uvm_mmap(&vms->vm_map, &vaddr, size,
+	    UVM_PROT_READ | UVM_PROT_WRITE, UVM_PROT_ALL, MAP_SHARED,
+	    &vn->v_uobj, foff, p->p_rlimit[RLIMIT_MEMLOCK].rlim_cur);
 #endif /* __NetBSD__ */
+	if (retcode)
+		goto done;
+
+	request.virtual = (void *)vaddr;
+
+	for ( i = 0 ; i < dma->buf_count ; i++ ) {
+		if (DRM_COPY_TO_USER(&request.list[i].idx,
+		    &dma->buflist[i]->idx, sizeof(request.list[0].idx))) {
+			retcode = EFAULT;
+			goto done;
 		}
-		if (retcode)
+		if (DRM_COPY_TO_USER(&request.list[i].total,
+		    &dma->buflist[i]->total, sizeof(request.list[0].total))) {
+			retcode = EFAULT;
+			goto done;
+		}
+		if (DRM_COPY_TO_USER(&request.list[i].used, &zero,
+		    sizeof(zero))) {
+			retcode = EFAULT;
+			goto done;
+		}
+		address = vaddr + dma->buflist[i]->offset; /* *** */
+		if (DRM_COPY_TO_USER(&request.list[i].address, &address,
+		    sizeof(address))) {
+			retcode = EFAULT;
 			goto done;
-		request.virtual = (void *)virtual;
-
-		for ( i = 0 ; i < dma->buf_count ; i++ ) {
-			if ( DRM_COPY_TO_USER( &request.list[i].idx,
-					   &dma->buflist[i]->idx,
-					   sizeof(request.list[0].idx) ) ) {
-				retcode = EFAULT;
-				goto done;
-			}
-			if ( DRM_COPY_TO_USER( &request.list[i].total,
-					   &dma->buflist[i]->total,
-					   sizeof(request.list[0].total) ) ) {
-				retcode = EFAULT;
-				goto done;
-			}
-			if ( DRM_COPY_TO_USER( &request.list[i].used,
-					   &zero,
-					   sizeof(zero) ) ) {
-				retcode = EFAULT;
-				goto done;
-			}
-			address = virtual + dma->buflist[i]->offset; /* *** */
-			if ( DRM_COPY_TO_USER( &request.list[i].address,
-					   &address,
-					   sizeof(address) ) ) {
-				retcode = EFAULT;
-				goto done;
-			}
 		}
 	}
+
  done:
 	request.count = dma->buf_count;
 
 	DRM_DEBUG( "%d buffers, retcode = %d\n", request.count, retcode );
 
-	DRM_COPY_TO_USER_IOCTL( (drm_buf_map_t *)data, request, sizeof(request) );
+	DRM_COPY_TO_USER_IOCTL((drm_buf_map_t *)data, request, sizeof(request));
 
 	return DRM_ERR(retcode);
 }
diff --git a/bsd/drm_drv.h b/bsd/drm_drv.h
index 66917668..d517d056 100644
--- a/bsd/drm_drv.h
+++ b/bsd/drm_drv.h
@@ -853,7 +853,7 @@ int DRM(close)(dev_t kdev, int flags, int fmt, DRM_STRUCTPROC *p)
 	drm_file_t *priv;
 	DRM_DEVICE;
 	int retcode = 0;
-	DRMFILE __unused filp = (void *)(DRM_CURRENTPID);
+	DRMFILE filp = (void *)(DRM_CURRENTPID);
 	
 	DRM_DEBUG( "open_count = %d\n", dev->open_count );
 	priv = DRM(find_file_by_proc)(dev, p);
@@ -877,7 +877,7 @@ int DRM(close)(dev_t kdev, int flags, int fmt, DRM_STRUCTPROC *p)
 #endif
 
 	if (dev->lock.hw_lock && _DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock)
-	    && dev->lock.filp == (void *)DRM_CURRENTPID) {
+	    && dev->lock.filp == filp) {
 		DRM_DEBUG("Process %d dead, freeing lock for context %d\n",
 			  DRM_CURRENTPID,
 			  _DRM_LOCKING_CONTEXT(dev->lock.hw_lock->lock));
@@ -1019,22 +1019,21 @@ int DRM(ioctl)(dev_t kdev, u_long cmd, caddr_t data, int flags,
 #endif /* __NetBSD__ */
 	}
 
-	if ( nr >= DRIVER_IOCTL_COUNT ) {
-		retcode = EINVAL;
-	} else {
-		ioctl = &DRM(ioctls)[nr];
-		func = ioctl->func;
-
-		if ( !func ) {
-			DRM_DEBUG( "no function\n" );
-			retcode = EINVAL;
-		} else if ( ( ioctl->root_only && DRM_SUSER(p) ) 
-			 || ( ioctl->auth_needed && !priv->authenticated ) ) {
-			retcode = EACCES;
-		} else {
-			retcode = func(kdev, cmd, data, flags, p, (void *)DRM_CURRENTPID);
-		}
+	if (nr >= DRIVER_IOCTL_COUNT)
+		return EINVAL;
+
+	ioctl = &DRM(ioctls)[nr];
+	func = ioctl->func;
+
+	if (func == NULL) {
+		DRM_DEBUG( "no function\n" );
+		return EINVAL;
 	}
+	if ((ioctl->root_only && DRM_SUSER(p)) || (ioctl->auth_needed &&
+	    !priv->authenticated))
+		return EACCES;
+
+	retcode = func(kdev, cmd, data, flags, p, (void *)DRM_CURRENTPID);
 
 	return DRM_ERR(retcode);
 }
@@ -1062,44 +1061,39 @@ int DRM(lock)( DRM_IOCTL_ARGS )
                 return DRM_ERR(EINVAL);
 #endif
 
-        if ( !ret ) {
-                for (;;) {
-                        if ( !dev->lock.hw_lock ) {
-                                /* Device has been unregistered */
-                                ret = EINTR;
-                                break;
-                        }
-                        if ( DRM(lock_take)( &dev->lock.hw_lock->lock,
-					     lock.context ) ) {
-                                dev->lock.filp = (void *)DRM_CURRENTPID;
-                                dev->lock.lock_time = jiffies;
-                                atomic_inc( &dev->counts[_DRM_STAT_LOCKS] );
-                                break;  /* Got lock */
-                        }
+	for (;;) {
+		if (dev->lock.hw_lock == NULL) {
+			/* Device has been unregistered */
+			ret = DRM_ERR(EINTR);
+			break;
+		}
+		if (DRM(lock_take)(&dev->lock.hw_lock->lock, lock.context)) {
+			dev->lock.filp = (void *)DRM_CURRENTPID;
+			dev->lock.lock_time = jiffies;
+			atomic_inc(&dev->counts[_DRM_STAT_LOCKS]);
+			break;  /* Got lock */
+		}
 
-                                /* Contention */
-			ret = tsleep((void *)&dev->lock.lock_queue,
-					PZERO|PCATCH,
-					"drmlk2",
-					0);
-			if (ret)
-				break;
-                }
-        }
+		/* Contention */
+		ret = tsleep((void *)&dev->lock.lock_queue, PZERO|PCATCH,
+		    "drmlk2", 0);
+		if (ret != 0)
+			break;
+	}
+	DRM_DEBUG( "%d %s\n", lock.context, ret ? "interrupted" : "has lock" );
+
+	if (ret != 0)
+		return ret;
 
-        if ( !ret ) {
-		/* FIXME: Add signal blocking here */
+	/* XXX: Add signal blocking here */
 
 #if __HAVE_DMA_QUIESCENT
-                if ( lock.flags & _DRM_LOCK_QUIESCENT ) {
-			DRIVER_DMA_QUIESCENT();
-		}
+	if (lock.flags & _DRM_LOCK_QUIESCENT) {
+		DRIVER_DMA_QUIESCENT();
+	}
 #endif
-        }
 
-        DRM_DEBUG( "%d %s\n", lock.context, ret ? "interrupted" : "has lock" );
-
-	return DRM_ERR(ret);
+	return 0;
 }