devsegment: cleanups

- return distinct error message when attempting to create Globalseg with physseg name
- copy directory name to up->genbuf so it stays valid after we unlock(&glogalseglock)
- cleanup wstat() handling, allow changing uid
- make sure global segment size is below SEGMAXSIZE
- move isoverlap() check from globalsegattach() into segattach()
- remove Proc* argument from globalsegattach(), segattach() and isoverlap()
- make Physseg.attr and segattach attr parameter an int for consistency

5 files changed, 62 insertions, 61 deletions

diff --git a/sys/src/9/port/devsegment.c b/sys/src/9/port/devsegment.c
index b712e1878..4f0e6664d 100644
--- a/sys/src/9/port/devsegment.c
+++ b/sys/src/9/port/devsegment.c
@@ -34,8 +34,8 @@ static Globalseg *globalseg[100];
 static Lock globalseglock;
 
 
-	Segment* (*_globalsegattach)(Proc*, char*);
-static	Segment* globalsegattach(Proc *p, char *name);
+	Segment* (*_globalsegattach)(char*);
+static	Segment* globalsegattach(char *name);
 
 static	Segment* fixedseg(uintptr va, ulong len);
 
@@ -64,7 +64,7 @@ getgseg(Chan *c)
 static void
 putgseg(Globalseg *g)
 {
-	if(decref(g) > 0)
+	if(g == nil || decref(g))
 		return;
 	if(g->s != nil)
 		putseg(g->s);
@@ -103,9 +103,9 @@ segmentgen(Chan *c, char*, Dirtab*, int, int s, Dir *dp)
 		q.vers = 0;
 		q.path = PATH(s, Qsegdir);
 		q.type = QTDIR;
-		devdir(c, q, g->name, 0, g->uid, DMDIR|0777, dp);
+		kstrcpy(up->genbuf, g->name, sizeof up->genbuf);
+		devdir(c, q, up->genbuf, 0, g->uid, DMDIR|0777, dp);
 		unlock(&globalseglock);
-
 		break;
 	case Qsegdir:
 		if(s == DEVDOTDOT){
@@ -183,16 +183,6 @@ segmentopen(Chan *c, int omode)
 			error(Eisdir);
 		break;
 	case Qctl:
-		g = getgseg(c);
-		if(waserror()){
-			putgseg(g);
-			nexterror();
-		}
-		devpermcheck(g->uid, g->perm, omode);
-		c->aux = g;
-		poperror();
-		c->flag |= COPEN;
-		break;
 	case Qdata:
 		g = getgseg(c);
 		if(waserror()){
@@ -200,27 +190,31 @@ segmentopen(Chan *c, int omode)
 			nexterror();
 		}
 		devpermcheck(g->uid, g->perm, omode);
-		if(g->s == nil)
+		if(TYPE(c) == Qdata && g->s == nil)
 			error("segment not yet allocated");
 		c->aux = g;
 		poperror();
-		c->flag |= COPEN;
 		break;
 	default:
 		panic("segmentopen");
 	}
 	c->mode = openmode(omode);
 	c->offset = 0;
+	c->flag |= COPEN;
 	return c;
 }
 
 static void
 segmentclose(Chan *c)
 {
-	if(TYPE(c) == Qtopdir)
-		return;
-	if(c->flag & COPEN)
-		putgseg(c->aux);
+	switch(TYPE(c)){
+	case Qctl:
+	case Qdata:
+		if(c->flag & COPEN){
+			putgseg(c->aux);
+			c->aux = nil;
+		}
+	}
 }
 
 static Chan*
@@ -232,8 +226,11 @@ segmentcreate(Chan *c, char *name, int omode, ulong perm)
 	if(TYPE(c) != Qtopdir)
 		error(Eperm);
 
+	if(strlen(name) >= sizeof(up->genbuf))
+		error(Etoolong);
+
 	if(findphysseg(name) != nil)
-		error(Eexist);
+		error("name collision with physical segment");
 
 	if((perm & DMDIR) == 0)
 		error(Ebadarg);
@@ -249,10 +246,8 @@ segmentcreate(Chan *c, char *name, int omode, ulong perm)
 		if(g == nil){
 			if(xfree < 0)
 				xfree = x;
-		} else {
-			if(strcmp(g->name, name) == 0)
-				error(Eexist);
-		}
+		} else if(strcmp(g->name, name) == 0)
+			error(Eexist);
 	}
 	if(xfree < 0)
 		error("too many global segments");
@@ -269,7 +264,6 @@ segmentcreate(Chan *c, char *name, int omode, ulong perm)
 	c->qid.type = QTDIR;
 	c->qid.vers = 0;
 	c->mode = openmode(omode);
-	c->mode = OWRITE;
 	return c;
 }
 
@@ -330,13 +324,15 @@ segmentwrite(Chan *c, void *a, long n, vlong voff)
 			va = va&~(BY2PG-1);
 			if(va == 0 || top > USTKTOP || top <= va)
 				error(Ebadarg);
-			len = (top - va) / BY2PG;
+			len = top - va;
+			if(len > SEGMAXSIZE)
+				error(Enovmem);
 			if(cb->nf >= 4 && strcmp(cb->f[3], "fixed") == 0){
 				if(!iseve())
 					error(Eperm);
-				g->s = fixedseg(va, len);
+				g->s = fixedseg(va, len/BY2PG);
 			} else
-				g->s = newseg(SG_SHARED, va, len);
+				g->s = newseg(SG_SHARED, va, len/BY2PG);
 		} else
 			error(Ebadctl);
 		free(cb);
@@ -359,22 +355,27 @@ segmentwstat(Chan *c, uchar *dp, int n)
 	if(c->qid.type == QTDIR)
 		error(Eperm);
 
+	d = nil;
 	g = getgseg(c);
 	if(waserror()){
+		free(d);
 		putgseg(g);
 		nexterror();
 	}
-
 	if(strcmp(g->uid, up->user) && !iseve())
 		error(Eperm);
 	d = smalloc(sizeof(Dir)+n);
 	n = convM2D(dp, n, &d[0], (char*)&d[1]);
-	g->perm = d->mode & 0777;
-
+	if(n == 0)
+		error(Eshortstat);
+	if(!emptystr(d->uid))
+		kstrdup(&g->uid, d->uid);
+	if(d->mode != ~0UL)
+		g->perm = d->mode&0777;
+	free(d);
 	putgseg(g);
 	poperror();
 
-	free(d);
 	return n;
 }
 
@@ -399,13 +400,12 @@ segmentremove(Chan *c)
  *  called by segattach()
  */
 static Segment*
-globalsegattach(Proc *p, char *name)
+globalsegattach(char *name)
 {
 	int x;
 	Globalseg *g;
 	Segment *s;
 
-	g = nil;
 	if(waserror()){
 		unlock(&globalseglock);
 		nexterror();
@@ -414,19 +414,16 @@ globalsegattach(Proc *p, char *name)
 	for(x = 0; x < nelem(globalseg); x++){
 		g = globalseg[x];
 		if(g != nil && strcmp(g->name, name) == 0)
-			break;
-	}
-	if(x == nelem(globalseg)){
-		unlock(&globalseglock);
-		poperror();
-		return nil;
+			goto Found;
 	}
+	unlock(&globalseglock);
+	poperror();
+	return nil;
+Found:
 	devpermcheck(g->uid, g->perm, ORDWR);
 	s = g->s;
 	if(s == nil)
 		error("global segment not assigned a virtual address");
-	if(isoverlap(p, s->base, s->top - s->base) != nil)
-		error("overlaps existing segment");
 	incref(s);
 	unlock(&globalseglock);
 	poperror();
diff --git a/sys/src/9/port/portdat.h b/sys/src/9/port/portdat.h
index 9d2337f5f..9e3b62c7f 100644
--- a/sys/src/9/port/portdat.h
+++ b/sys/src/9/port/portdat.h
@@ -389,7 +389,7 @@ enum
 
 struct Physseg
 {
-	ulong	attr;			/* Segment attributes */
+	int	attr;			/* Segment attributes */
 	char	*name;			/* Attach name */
 	uintptr	pa;			/* Physical address */
 	uintptr	size;			/* Maximum segment size in bytes */
diff --git a/sys/src/9/port/portfns.h b/sys/src/9/port/portfns.h
index 36eb2a0fd..4b639c3a1 100644
--- a/sys/src/9/port/portfns.h
+++ b/sys/src/9/port/portfns.h
@@ -140,7 +140,7 @@ int		iprint(char*, ...);
 void		isdir(Chan*);
 int		iseve(void);
 int		islo(void);
-Segment*	isoverlap(Proc*, uintptr, uintptr);
+Segment*	isoverlap(uintptr, uintptr);
 Physseg*	findphysseg(char*);
 void		ixsummary(void);
 void		kickpager(void);
@@ -309,7 +309,7 @@ void		scheddump(void);
 void		schedinit(void);
 void		(*screenputs)(char*, int);
 long		seconds(void);
-uintptr		segattach(Proc*, ulong, char *, uintptr, uintptr);
+uintptr		segattach(int, char *, uintptr, uintptr);
 void		segclock(uintptr);
 long		segio(Segio*, Segment*, void*, long, vlong, int);
 void		segpage(Segment*, Page*);
diff --git a/sys/src/9/port/segment.c b/sys/src/9/port/segment.c
index e257e47b3..e7f2f42b0 100644
--- a/sys/src/9/port/segment.c
+++ b/sys/src/9/port/segment.c
@@ -27,7 +27,7 @@ static struct Imagealloc
 	QLock	ireclaim;	/* mutex on reclaiming free images */
 }imagealloc;
 
-Segment* (*_globalsegattach)(Proc*, char*);
+Segment* (*_globalsegattach)(char*);
 
 void
 initseg(void)
@@ -533,7 +533,7 @@ mfreeseg(Segment *s, uintptr start, ulong pages)
 }
 
 Segment*
-isoverlap(Proc *p, uintptr va, uintptr len)
+isoverlap(uintptr va, uintptr len)
 {
 	int i;
 	Segment *ns;
@@ -541,7 +541,7 @@ isoverlap(Proc *p, uintptr va, uintptr len)
 
 	newtop = va+len;
 	for(i = 0; i < NSEG; i++) {
-		ns = p->seg[i];
+		ns = up->seg[i];
 		if(ns == nil)
 			continue;
 		if((newtop > ns->base && newtop <= ns->top) ||
@@ -590,7 +590,7 @@ findphysseg(char *name)
 }
 
 uintptr
-segattach(Proc *p, ulong attr, char *name, uintptr va, uintptr len)
+segattach(int attr, char *name, uintptr va, uintptr len)
 {
 	int sno;
 	Segment *s, *os;
@@ -600,7 +600,7 @@ segattach(Proc *p, ulong attr, char *name, uintptr va, uintptr len)
 		error(Ebadarg);
 
 	for(sno = 0; sno < NSEG; sno++)
-		if(p->seg[sno] == nil && sno != ESEG)
+		if(up->seg[sno] == nil && sno != ESEG)
 			break;
 
 	if(sno == NSEG)
@@ -611,9 +611,13 @@ segattach(Proc *p, ulong attr, char *name, uintptr va, uintptr len)
 	 *  same name
 	 */
 	if(_globalsegattach != nil){
-		s = (*_globalsegattach)(p, name);
+		s = (*_globalsegattach)(name);
 		if(s != nil){
-			p->seg[sno] = s;
+			if(isoverlap(s->base, s->top - s->base) != nil){
+				putseg(s);
+				error(Esoverlap);
+			}
+			up->seg[sno] = s;
 			return s->base;
 		}
 	}
@@ -634,7 +638,7 @@ segattach(Proc *p, ulong attr, char *name, uintptr va, uintptr len)
 	 * map the zero page.
 	 */
 	if(va == 0) {
-		for (os = p->seg[SSEG]; os != nil; os = isoverlap(p, va, len)) {
+		for (os = up->seg[SSEG]; os != nil; os = isoverlap(va, len)) {
 			va = os->base;
 			if(len >= va)
 				error(Enovmem);
@@ -646,7 +650,7 @@ segattach(Proc *p, ulong attr, char *name, uintptr va, uintptr len)
 	if(va == 0 || (va+len) > USTKTOP || (va+len) < va)
 		error(Ebadarg);
 
-	if(isoverlap(p, va, len) != nil)
+	if(isoverlap(va, len) != nil)
 		error(Esoverlap);
 
 	ps = findphysseg(name);
@@ -661,7 +665,7 @@ segattach(Proc *p, ulong attr, char *name, uintptr va, uintptr len)
 
 	s = newseg(attr, va, len/BY2PG);
 	s->pseg = ps;
-	p->seg[sno] = s;
+	up->seg[sno] = s;
 
 	return va;
 }
diff --git a/sys/src/9/port/sysproc.c b/sys/src/9/port/sysproc.c
index 267a4b20c..6cc23c265 100644
--- a/sys/src/9/port/sysproc.c
+++ b/sys/src/9/port/sysproc.c
@@ -429,7 +429,7 @@ sysexec(va_list list)
 		tstk = s->base;
 		if(tstk <= USTKSIZE)
 			error(Enovmem);
-	} while((s = isoverlap(up, tstk-USTKSIZE, USTKSIZE)) != nil);
+	} while((s = isoverlap(tstk-USTKSIZE, USTKSIZE)) != nil);
 	up->seg[ESEG] = newseg(SG_STACK, tstk-USTKSIZE, USTKSIZE/BY2PG);
 
 	/*
@@ -785,12 +785,12 @@ syssegbrk(va_list list)
 uintptr
 syssegattach(va_list list)
 {
-	ulong attr;
+	int attr;
 	char *name;
 	uintptr va;
 	ulong len;
 
-	attr = va_arg(list, ulong);
+	attr = va_arg(list, int);
 	name = va_arg(list, char*);
 	va = va_arg(list, uintptr);
 	len = va_arg(list, ulong);
@@ -800,7 +800,7 @@ syssegattach(va_list list)
 		free(name);
 		nexterror();
 	}
-	va = segattach(up, attr, name, va, len);
+	va = segattach(attr, name, va, len);
 	free(name);
 	poperror();
 	return va;