git: 9front

Download patch

ref: 1c0dc31af5cef7754824de665a6de4c80636ddc3
parent: c508c4532f2fbc637ab6623e6dc9fba7118997ab
author: cinap_lenrek <cinap_lenrek@gmx.de>
date: Mon Jun 17 17:58:38 EDT 2013 

devsrv: fix wstat(), srvname(), avoid smalloc() while holding srv qlock, style

- wstat would half update the Srv data structure if name was too long
- srvname() walked the linked srv list without holding srv qlock
- dont access sp->chan while not holding srv qlock in srvopen()
- dont modify sp->owner while not holding srv qlock in srvcreate()
- avoid smalloc() allocations while holding srv qlock
- style pikeshedding, sorry


--- a/sys/src/9/port/devsrv.c
+++ b/sys/src/9/port/devsrv.c
@@ -25,9 +25,11 @@
 srvlookup(char *name, ulong qidpath)
 {
 	Srv *sp;
-	for(sp = srv; sp; sp = sp->link)
-		if(sp->path == qidpath || (name && strcmp(sp->name, name) == 0))
+
+	for(sp = srv; sp != nil; sp = sp->link) {
+		if(sp->path == qidpath || (name != nil && strcmp(sp->name, name) == 0))
 			return sp;
+	}
 	return nil;
 }
 
@@ -43,13 +45,13 @@
 	}
 
 	qlock(&srvlk);
-	if(name)
+	if(name != nil)
 		sp = srvlookup(name, -1);
 	else {
-		for(sp = srv; sp && s; sp = sp->link)
+		for(sp = srv; sp != nil && s > 0; sp = sp->link)
 			s--;
 	}
-	if(sp == 0 || name && (strlen(sp->name) >= sizeof(up->genbuf))) {
+	if(sp == nil || (name != nil && (strlen(sp->name) >= sizeof(up->genbuf)))) {
 		qunlock(&srvlk);
 		return -1;
 	}
@@ -91,13 +93,18 @@
 	Srv *sp;
 	char *s;
 
-	for(sp = srv; sp; sp = sp->link)
+	s = nil;
+	qlock(&srvlk);
+	for(sp = srv; sp != nil; sp = sp->link) {
 		if(sp->chan == c){
-			s = smalloc(3+strlen(sp->name)+1);
-			sprint(s, "#s/%s", sp->name);
-			return s;
+			s = malloc(3+strlen(sp->name)+1);
+			if(s != nil)
+				sprint(s, "#s/%s", sp->name);
+			break;
 		}
-	return nil;
+	}
+	qunlock(&srvlk);
+	return s;
 }
 
 static Chan*
@@ -104,6 +111,7 @@
 srvopen(Chan *c, int omode)
 {
 	Srv *sp;
+	Chan *nc;
 
 	if(c->qid.type == QTDIR){
 		if(omode & ORCLOSE)
@@ -122,7 +130,7 @@
 	}
 
 	sp = srvlookup(nil, c->qid.path);
-	if(sp == 0 || sp->chan == 0)
+	if(sp == nil || sp->chan == nil)
 		error(Eshutdown);
 
 	if(omode&OTRUNC)
@@ -131,17 +139,19 @@
 		error(Eperm);
 	devpermcheck(sp->owner, sp->perm, omode);
 
-	cclose(c);
-	incref(sp->chan);
+	nc = sp->chan;
+	incref(nc);
+
 	qunlock(&srvlk);
 	poperror();
-	return sp->chan;
+
+	cclose(c);
+	return nc;
 }
 
 static Chan*
 srvcreate(Chan *c, char *name, int omode, ulong perm)
 {
-	char *sname;
 	Srv *sp;
 
 	if(openmode(omode) != OWRITE)
@@ -151,35 +161,35 @@
 		error(Etoolong);
 
 	sp = smalloc(sizeof *sp);
-	sname = smalloc(strlen(name)+1);
+	kstrdup(&sp->name, name);
+	kstrdup(&sp->owner, up->user);
 
 	qlock(&srvlk);
 	if(waserror()){
-		free(sp);
-		free(sname);
 		qunlock(&srvlk);
+		free(sp->owner);
+		free(sp->name);
+		free(sp);
 		nexterror();
 	}
-	if(sp == nil || sname == nil)
-		error(Enomem);
-	if(srvlookup(name, -1))
+	if(srvlookup(name, -1) != nil)
 		error(Eexist);
 
+	sp->perm = perm&0777;
 	sp->path = qidpath++;
-	sp->link = srv;
-	strcpy(sname, name);
-	sp->name = sname;
-	c->qid.type = QTFILE;
+
 	c->qid.path = sp->path;
+	c->qid.type = QTFILE;
+
+	sp->link = srv;
 	srv = sp;
+
 	qunlock(&srvlk);
 	poperror();
 
-	kstrdup(&sp->owner, up->user);
-	sp->perm = perm&0777;
-
 	c->flag |= COPEN;
 	c->mode = OWRITE;
+
 	return c;
 }
 
@@ -197,13 +207,12 @@
 		nexterror();
 	}
 	l = &srv;
-	for(sp = *l; sp; sp = sp->link) {
+	for(sp = *l; sp != nil; sp = *l) {
 		if(sp->path == c->qid.path)
 			break;
-
 		l = &sp->link;
 	}
-	if(sp == 0)
+	if(sp == nil)
 		error(Enonexist);
 
 	/*
@@ -219,10 +228,12 @@
 		error(Eperm);
 
 	*l = sp->link;
+	sp->link = nil;
+
 	qunlock(&srvlk);
 	poperror();
 
-	if(sp->chan)
+	if(sp->chan != nil)
 		cclose(sp->chan);
 	free(sp->owner);
 	free(sp->name);
@@ -233,36 +244,37 @@
 srvwstat(Chan *c, uchar *dp, int n)
 {
 	char *strs;
-	Dir d;
 	Srv *sp;
+	Dir d;
 
 	if(c->qid.type & QTDIR)
 		error(Eperm);
 
-	strs = nil;
+	strs = smalloc(n);
+	if(waserror()){
+		free(strs);
+		nexterror();
+	}
+	n = convM2D(dp, n, &d, strs);
+	if(n == 0)
+		error(Eshortstat);
+
 	qlock(&srvlk);
 	if(waserror()){
 		qunlock(&srvlk);
-		free(strs);
 		nexterror();
 	}
 
 	sp = srvlookup(nil, c->qid.path);
-	if(sp == 0)
+	if(sp == nil)
 		error(Enonexist);
 
 	if(strcmp(sp->owner, up->user) != 0 && !iseve())
 		error(Eperm);
 
-	strs = smalloc(n);
-	n = convM2D(dp, n, &d, strs);
-	if(n == 0)
-		error(Eshortstat);
 	if(d.mode != ~0UL)
 		sp->perm = d.mode & 0777;
-	if(d.uid && *d.uid)
-		kstrdup(&sp->owner, d.uid);
-	if(d.name && *d.name && strcmp(sp->name, d.name) != 0) {
+	if(d.name != nil && *d.name && strcmp(sp->name, d.name) != 0) {
 		if(strchr(d.name, '/') != nil)
 			error(Ebadchar);
 		if(strlen(d.name) >= sizeof(up->genbuf))
@@ -269,9 +281,15 @@
 			error(Etoolong);
 		kstrdup(&sp->name, d.name);
 	}
+	if(d.uid != nil && *d.uid)
+		kstrdup(&sp->owner, d.uid);
+
 	qunlock(&srvlk);
+	poperror();
+
 	free(strs);
 	poperror();
+
 	return n;
 }
 
@@ -325,13 +343,14 @@
 	if(c1->qid.type & QTAUTH)
 		error("cannot post auth file in srv");
 	sp = srvlookup(nil, c->qid.path);
-	if(sp == 0)
+	if(sp == nil)
 		error(Enonexist);
 
-	if(sp->chan)
+	if(sp->chan != nil)
 		error(Ebadusefd);
 
 	sp->chan = c1;
+
 	qunlock(&srvlk);
 	poperror();
 	return n;
@@ -364,8 +383,9 @@
 	Srv *sp;
 
 	qlock(&srvlk);
-	for(sp = srv; sp; sp = sp->link)
-		if(sp->owner!=nil && strcmp(old, sp->owner)==0)
+	for(sp = srv; sp != nil; sp = sp->link) {
+		if(sp->owner != nil && strcmp(old, sp->owner) == 0)
 			kstrdup(&sp->owner, new);
+	}
 	qunlock(&srvlk);
 }
-- 
⑨