PERFORCE change 127995 for review

[Marko Zec]

http://perforce.freebsd.org/chv.cgi?CH=127995

Change 127995 by zec at zec_tpx32 on 2007/10/24 08:43:01

	Rewrite a function for constructing full path name to a
	vimage, which was previously not only yielding bogus
	results, but was recursively invoking itself, which was
	not a wise approach given the limited size of the kernel
	stack.
	
	Rewrite vimage_get_next() function so that it doesn't
	traverse through the entire list of vimages, but walks
	the hierarchy by looking at child/parent/sibling relations.
	
	Change the convention for addressing parent vnet when
	returning an ifnet back to parent: instead of "-" now
	".." is used.
	
	Fix spelling: vi_sibilings -> vi_sibling
	
	Do not expose vimage ID to userspace any more, since only
	symbolic vimage name is relevant for addressing purposes.

Affected files ...

.. //depot/projects/vimage/src/sys/kern/kern_vimage.c#51 edit
.. //depot/projects/vimage/src/sys/sys/vimage.h#48 edit

Differences ...

==== //depot/projects/vimage/src/sys/kern/kern_vimage.c#51 (text+ko) ====

@@ -285,7 +285,7 @@
  * passed as ifp.  The interface will be renamed to vi_req->vi_parent_name
  * if vi_req->vi_parent_name is not an empty string (uff ugly ugly)...
  * Similary, the target vnet can be specified either by vnet argument or
- * by name. If vnet name equals to "-" or vi_req is set to NULL the
+ * by name. If vnet name equals to ".." or vi_req is set to NULL the
  * interface is moved to the parent vnet.
  */
 int
@@ -298,7 +298,7 @@
 	struct vnet *new_vnet = NULL;
 	u_char eaddr[6];
 
-	if (vi_req == NULL || strcmp(vi_req->vi_name, "-") == 0) {
+	if (vi_req == NULL || strcmp(vi_req->vi_name, "..") == 0) {
 		if (IS_DEFAULT_VIMAGE(vip))
 			return (ENXIO);
 		new_vnet = vip->vi_parent->v_net;
@@ -465,7 +465,7 @@
 		namelen = strlen(name);
 	if (namelen == 0)
 		return(NULL);
-	LIST_FOREACH(vip, &top->vi_child_head, vi_sibilings)
+	LIST_FOREACH(vip, &top->vi_child_head, vi_sibling)
 		if (strncmp(name, vip->vi_name, namelen) == 0) {
 			if (next_name != NULL)
 				return(vimage_by_name(vip, next_name));
@@ -476,40 +476,58 @@
 }
 
 
-static int
+static void
 vimage_relative_name(struct vimage *top, struct vimage *where,
     char *buffer, int bufflen)
 {
+	int used = 1;
+
 	if (where == top) {
 		sprintf(buffer, ".");
-		return(1);
-	}
+		return;
+	} else
+		*buffer = 0;
 
-	if (where->vi_parent != top) {
-		int len;
+	do {
+		int namelen = strlen(where->vi_name);
 
-		len  = vimage_relative_name(top, where->vi_parent,
-		    buffer, bufflen);
-		bufflen -= (len + 1);
-		buffer += len;
-		sprintf(buffer++, ".");
-	}
+		if (namelen + used + 1 >= bufflen)
+			panic("buffer overflow");
 
-	sprintf(buffer, "%s", where->vi_name);
-	return(strlen(where->vi_name));
+		if (used > 1) {
+			bcopy(buffer, &buffer[namelen + 1], used);
+			buffer[namelen] = '.';
+			used++;
+		} else
+			bcopy(buffer, &buffer[namelen], used);
+		bcopy(where->vi_name, buffer, namelen);
+		used += namelen;
+		where = where->vi_parent;
+	} while (where != top);
 }
 
 
 static struct vimage *
 vimage_get_next(struct vimage *top, struct vimage *where)
 {
+	struct vimage *next;
+
+	/* Try to go deeper in the hierarchy */
+	next = LIST_FIRST(&where->vi_child_head);
+	if (next != NULL)
+		return(next);
+
 	do {
-		where = LIST_NEXT(where, vi_le);
-		if (where == NULL)
-			where = LIST_FIRST(&vimage_head);
-		if (vi_child_of(top, where))
-			return(where);
-	} while (where != top);
+		/* Try to find next sibling */
+		next = LIST_NEXT(where, vi_sibling);
+		if (next != NULL)
+			return(next);
+
+		/* Nothing left on this level, go one level up */
+		where = where->vi_parent;
+	} while (where != top->vi_parent);
+
+	/* Nothing left to be visited, we are done */
 	return(NULL);
 }
 
@@ -546,7 +564,6 @@
 	switch (cmd) {
 
 	case SIOCGPVIMAGE:
-		vi_req->vi_id = vip_r->vi_id;
 		vimage_relative_name(vip, vip_r, vi_req->vi_name,
 		    sizeof (vi_req->vi_name));
 		bcopy(&vip_r->v_procg->_averunnable, &vi_req->averunnable,
@@ -675,7 +692,7 @@
 	vip->vi_parent = parent;
 	/* XXX locking */
 	if (parent != NULL)
-		LIST_INSERT_HEAD(&parent->vi_child_head, vip, vi_sibilings);
+		LIST_INSERT_HEAD(&parent->vi_child_head, vip, vi_sibling);
 	else if (!LIST_EMPTY(&vimage_head))
 		panic("there can be only one default vimage!");
 	LIST_INSERT_HEAD(&vimage_head, vip, vi_le);
@@ -755,7 +772,7 @@
 	/* Point with no return - cleanup MUST succeed! */
 	/* XXX locking */
 	LIST_REMOVE(vip, vi_le);
-	LIST_REMOVE(vip, vi_sibilings);
+	LIST_REMOVE(vip, vi_sibling);
 
 	/* XXX locking */
 	LIST_REMOVE(vprocg, vprocg_le);

==== //depot/projects/vimage/src/sys/sys/vimage.h#48 (text+ko) ====

@@ -379,7 +379,7 @@
 
 struct vimage {
 	LIST_ENTRY(vimage) vi_le;	/* all vimage list */
-	LIST_ENTRY(vimage) vi_sibilings;	/* vimages with same parent */
+	LIST_ENTRY(vimage) vi_sibling;	/* vimages with same parent */
 	LIST_HEAD(, vimage) vi_child_head;	/* direct offspring list */
 	struct	vimage *vi_parent;	/* ptr to parent vimage */
 	u_int	vi_id;			/* ID num */
@@ -461,7 +461,6 @@
 	u_int	vi_cpu_weight;		/* Prop. share scheduling priority */
 	int	vi_intr_limit;		/* Limit on CPU usage in intr ctx */
 	int	vi_maxsockets;
-	u_short vi_id;			/* IDnum - but do we need it at all? */
 	u_short vi_proc_limit;		/* max. number of processes */
 	u_short vi_proc_count;		/* current number of processes */
 	u_short vi_child_limit;		/* max. number of child vnets */