PERFORCE change 174089 for review

[Robert Watson]

http://p4web.freebsd.org/chv.cgi?CH=174089

Change 174089 by rwatson at rwatson_vimage_client on 2010/02/01 14:17:02

	Clean up style in a few places.
	
	Properly handle lock unwinding in append.  Comment on lock
	recursion.

Affected files ...

.. //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_fdlist.c#8 edit

Differences ...

==== //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_fdlist.c#8 (text+ko) ====

@@ -31,7 +31,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_fdlist.c#7 $
+ * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_fdlist.c#8 $
  */
 
 #include <sys/mman.h>
@@ -47,31 +47,22 @@
 
 #include "libcapsicum_sandbox_api.h"
 
-
 struct lc_fdlist_entry {
-
-	unsigned int sysoff;	/* offset of e.g. "org.freebsd.rtld-elf-cap" */
-	unsigned int syslen;	/* length of above */
-
-	unsigned int classoff;	/* offset of variable ID e.g. "libs" */
-	unsigned int classnamelen;	/* length of above */
-
-	unsigned int nameoff;	/* offset of entry name (e.g. "libc.so.7") */
-	unsigned int namelen;	/* length of above */
-
+	u_int sysoff;		/* offset of e.g. "org.freebsd.rtld-elf-cap" */
+	u_int syslen;		/* length of above */
+	u_int classoff;		/* offset of variable ID e.g. "libs" */
+	u_int classnamelen;	/* length of above */
+	u_int nameoff;		/* offset of entry name (e.g. "libc.so.7") */
+	u_int namelen;		/* length of above */
 	int fd;			/* the file descriptor */
 };
 
-
 struct lc_fdlist_storage {
-
-	unsigned int count;		/* number of entries */
-	unsigned int capacity;		/* entries that we can hold */
-
-	unsigned int namelen;		/* bytes of name data */
-	unsigned int namecapacity;	/* bytes of name data we can hold */
-
-	struct lc_fdlist_entry entries[];	/* entries in the descriptor list */
+	u_int count;		/* number of entries */
+	u_int capacity;		/* entries that we can hold */
+	u_int namelen;		/* bytes of name data */
+	u_int namecapacity;	/* bytes of name data we can hold */
+	struct lc_fdlist_entry entries[]; /* entries in the descriptor list */
 
 	/* followed by bytes of name data */
 };
@@ -88,7 +79,6 @@
 static char	*lc_fdlist_storage_names(struct lc_fdlist_storage *lfsp);
 static u_int	 lc_fdlist_storage_size(struct lc_fdlist_storage *lfsp);
 
-
 static struct lc_fdlist global_fdlist = {
 	.lf_lock = PTHREAD_MUTEX_INITIALIZER,
 };
@@ -145,11 +135,9 @@
 	return (NULL);
 }
 
-
 #define INITIAL_ENTRIES		16
 #define INITIAL_NAMEBYTES	(64 * INITIAL_ENTRIES)
 
-
 struct lc_fdlist *
 lc_fdlist_new(void)
 {
@@ -296,19 +284,19 @@
 	char *head = names + lfsp->namelen;
 
 	strncpy(head, subsystem, subsyslen + 1);
-	entry->sysoff	= (head - names);
-	entry->syslen	= subsyslen;
-	head		+= subsyslen + 1;
+	entry->sysoff = (head - names);
+	entry->syslen = subsyslen;
+	head += subsyslen + 1;
 
 	strncpy(head, classname, classnamelen + 1);
 	entry->classoff	= (head - names);
-	entry->classnamelen	= classnamelen;
-	head		+= classnamelen + 1;
+	entry->classnamelen = classnamelen;
+	head += classnamelen + 1;
 
 	strncpy(head, name, namelen + 1);
-	entry->nameoff	= (head - names);
-	entry->namelen	= namelen + 1;
-	head		+= namelen + 1;
+	entry->nameoff = (head - names);
+	entry->namelen = namelen + 1;
+	head += namelen + 1;
 
 	lfsp->count++;
 	lfsp->namelen = (head - names);
@@ -331,27 +319,34 @@
 		LOCK(to);
 	}
 
-	for (unsigned int i = 0; i < from->lf_storage->count; i++) {
+	for (u_int i = 0; i < from->lf_storage->count; i++) {
 		char *subsystem;
 		char *classname;
 		char *name;
 		int fd;
 
+		/*
+		 * XXXRW: This recurses the from lock.
+		 */
 		if (lc_fdlist_getentry(from, &subsystem, &classname, &name,
 		    &fd, &pos) < 0)
-			return (-1);
+			goto fail;
 
+		/*
+		 * XXXRW: This recurses the to lock.
+		 */
 		if (lc_fdlist_add(to, subsystem, classname, name, fd) < 0) {
 			free(subsystem);
-			return (-1);
+			goto fail;
 		}
-
 		free(subsystem);
 	}
+	return (0);
 
+fail:
 	UNLOCK(from);
 	UNLOCK(to);
-	return (0);
+	return (-1);
 }
 
 int
@@ -389,7 +384,7 @@
 	int successful = 0;
 	const char *names = lc_fdlist_storage_names(lfsp);
 
-	for (unsigned int i = (pos ? *pos : 0); i < lfsp->count; i++) {
+	for (u_int i = (pos ? *pos : 0); i < lfsp->count; i++) {
 		struct lc_fdlist_entry *entry = lfsp->entries + i;
 
 		if ((!subsystem ||
@@ -406,16 +401,12 @@
 			}
 
 			*fdp = entry->fd;
-
 			if (pos) *pos = i + 1;
 			successful = 1;
-
 			break;
 		}
 	}
-
 	UNLOCK(lfp);
-
 	if (successful)
 		return (0);
 	errno = ENOENT;
@@ -455,12 +446,10 @@
 	head += size;
 
 	*fdp = entry->fd;
-
 	UNLOCK(lfp);
 
 	(*pos)++;
-
-	return 0;
+	return (0);
 }
 
 int
@@ -483,7 +472,7 @@
 	 * we care about.
 	 */
 	int highestfd = -1;
-	for (unsigned int i = 0; i < lfsp->count; i++) {
+	for (u_int i = 0; i < lfsp->count; i++) {
 		if (lfsp->entries[i].fd > highestfd)
 			highestfd = lfsp->entries[i].fd;
 	}
@@ -492,7 +481,7 @@
 	/*
 	 * First, move all our descriptors up the range.
 	 */
-	for (unsigned int i = 0; i < lfsp->count; i++) {
+	for (u_int i = 0; i < lfsp->count; i++) {
 		if (dup2(lfsp->entries[i].fd, highestfd + i) < 0) {
 			UNLOCK(lfp);
 			return (-1);
@@ -502,7 +491,7 @@
 	/*
 	 * Now put them back.
 	 */
-	for (unsigned int i = 0; i < lfsp->count; i++) {
+	for (u_int i = 0; i < lfsp->count; i++) {
 		if (dup2(highestfd + i, i) < 0) {
 			UNLOCK(lfp);
 			return (-1);
@@ -517,7 +506,6 @@
 	 */
 	closefrom(lfsp->count);
 	UNLOCK(lfp);
-
 	return (0);
 }