From owner-freebsd-arch Fri Nov 2 11:24:36 2001 Delivered-To: freebsd-arch@freebsd.org Received: from mail12.speakeasy.net (mail12.speakeasy.net [216.254.0.212]) by hub.freebsd.org (Postfix) with ESMTP id C7A2437B403 for ; Fri, 2 Nov 2001 11:24:31 -0800 (PST) Received: (qmail 99449 invoked from network); 2 Nov 2001 19:24:30 -0000 Received: from unknown (HELO laptop.baldwin.cx) ([64.81.54.73]) (envelope-sender ) by mail12.speakeasy.net (qmail-ldap-1.03) with SMTP for ; 2 Nov 2001 19:24:30 -0000 Message-ID: X-Mailer: XFMail 1.4.0 on FreeBSD X-Priority: 3 (Normal) Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 8bit MIME-Version: 1.0 In-Reply-To: Date: Fri, 02 Nov 2001 11:24:29 -0800 (PST) From: John Baldwin To: Kelly Yancey Subject: Re: Changes to suser() and friends Cc: arch@FreeBSD.ORG, Robert Watson , Julian Elischer , Garance A Drosihn Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On 01-Nov-01 Kelly Yancey wrote: > On Thu, 1 Nov 2001, Garance A Drosihn wrote: > >> Hmm. It is nicer when fewer source files are mucking around in the >> internals of a data structure. Still, it would be nice to collapse >> to fewer routines. Would the following be any better? >> >> int suser __P((struct ucred *cred, struct proc *proc, int flag)); >> int suser_td __P((struct ucred *cred, struct thread *thread, int flag)); >> >> and say that the current 'suser(struct proc *)' >> be done as new 'suser(NULL, struct proc *, 0)', >> and the current 'suser_td(struct thread *)' >> be done as new 'suser_td(NULL, struct thread *, 0)' >> or is that too stupid for words? Or maybe switch the positions of >> 'cred' and 'proc/thread', just so the most common callers would just >> have 'NULL, 0' as the last two parameters. Does that add too much >> overhead? >> > > I acknowledge I have no specific insight into suser(), but I would like to > add that if you are going to have two different behaviours keyed off of > whether a parameter is NULL or not, it is more logical to have two separate > functions representing those behaviours. See realloc as an example of what > happens when you overload a function with multiple behaviours keyed off of a > parameter value. This is presumably the exact sort of overloading that the > current (albeit poorly named) set of functions was intended to avoid. Agreed. After chatting a bit with rwatson on IRC, my conclusion is for the following two functions. I'm not sure worrying over includes is all that valuable, but having drivers and filesystems be able to handle threads and procs in the abstract instead of having to muck with their innards is cleaner. These two functions remove any ambiguity about what ucred is being used as well. int suser(struct thread *td, int flags) Uses td->td_proc->td_ucred for its ucred. Fairly soon it will switch to td->td_ucred when that is safe to do so. Note that this will only be useful for curthread. int suser_cred(struct ucred *cred, int flags) Uses cred for its ucred. Useful in places where you want to use a ucred other than a thread's ucred. suser() really is all about ucreds, so I can appreciate Robert's desire to have one function that just takes a ucred, but I like preserving the existing abstraction of d_thread_t. It will also make keeping code portable between 4.x and 5.x easier. -- John Baldwin -- http://www.FreeBSD.org/~jhb/ PGP Key: http://www.baldwin.cx/~john/pgpkey.asc "Power Users Use the Power to Serve!" - http://www.FreeBSD.org/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message