Date: Fri, 31 May 2002 10:23:19 -0700 (PDT) From: Julian Elischer <julian@elischer.org> To: Jake Burkholder <jake@locore.ca> Cc: Julian Elischer <julian@freebsd.org>, Perforce Change Reviews <perforce@freebsd.org> Subject: Re: PERFORCE change 12179 for review Message-ID: <Pine.BSF.4.21.0205311013490.28852-100000@InterJet.elischer.org> In-Reply-To: <20020531130656.T62759@locore.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, 31 May 2002, Jake Burkholder wrote: > Apparently, On Thu, May 30, 2002 at 10:57:02PM -0700, > Julian Elischer said words to the effect of; > > > http://people.freebsd.org/~peter/p4db/chv.cgi?CH=12179 > > > > Change 12179 by julian@julian_ref on 2002/05/30 22:56:19 > > > > Use fuword and suword instead of copyin/copyout > > when setting up thread completion mailboxes. > > needs fptr and sptr, which I will add soon. > > Please do not add these without discussing it first. fuword and > suword take a long as their argument/return value, which works > fine. except that you have to cast the results.. It'd be nice if there was one guaranteed to return a (void *) preferably at a second argument so that a success value can be given as well. (I decided not to add it myself unilaterally but rather to lobby for it amongst types such as yourself..) There is no guarantee that on every imaginable architecture that the same long<->pointer conversion is correct, and anyhow the casting is wrong.. it wouldn't take much to make a retval = fptr(addr, &target) call. > > > > > Affected files ... > > > > ... //depot/projects/kse/sys/i386/i386/trap.c#43 edit > > ... //depot/projects/kse/sys/kern/kern_thread.c#50 edit > > > > Differences ... > > > > ==== //depot/projects/kse/sys/i386/i386/trap.c#43 (text+ko) ==== > > > > @@ -955,10 +955,10 @@ > > * possibility that we could do this lazily (in sleep()), > > * but for now do it every time. > > */ > > - error = copyin((caddr_t)td->td_kse->ke_mailbox + > > - offsetof(struct kse_mailbox, current_thread), > > - &td->td_mailbox, sizeof(void *)); > > - if (error || td->td_mailbox == NULL) { > > + td->td_mailbox = fuword((caddr_t)td->td_kse->ke_mailbox + > > + offsetof(struct kse_mailbox, current_thread)); > > + if ((td->td_mailbox == NULL) || > > + (td->td_mailbox == (viod *)-1)) { > > td->td_mailbox = NULL; /* single thread it.. */ > > td->td_flags &= ~TDF_UNBOUND; > > } else { > > > > ==== //depot/projects/kse/sys/kern/kern_thread.c#50 (text+ko) ==== > > > > @@ -251,7 +251,7 @@ > > thread_export_context(struct thread *td) > > { > > struct kse *ke; > > - void *td2_mbx; > > + uint td2_mbx; /* XXXKSE */ > > sigh. sizeof(uint) != sizeof(void *). Use uintptr_t. That's one reason the XXXKSE is there.. I wasn't sure what to use... I'll fix it now.. > > > void *addr1; > > void *addr2; > > int error; > > @@ -266,11 +266,17 @@ > > + offsetof(struct thread_mailbox , next_completed); > > /* Then link it into it's KSE's list of completed threads. */ > > if (!error) > > - error = copyin( addr1, &td2_mbx, sizeof(void *)); > > + error = td2_mbx = fuword(addr1); > > + if (error == -1) > > + error = EFAULT; > > + else > > + error = 0; > > if (!error) > > - error = copyout(&td2_mbx, addr2, sizeof(void *)); > > + error = suword(addr2, td2_mbx); > > if (!error) > > - error = copyout(&td->td_mailbox, addr1, sizeof(void *)); > > + error = suword(addr1, (uint)td->td_mailbox); > > + if (error == -1) > > + error = EFAULT; > > return (error); > > } > > > To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe p4-projects" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?Pine.BSF.4.21.0205311013490.28852-100000>