Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 02 Nov 2001 11:24:29 -0800 (PST)
From:      John Baldwin <jhb@FreeBSD.org>
To:        Kelly Yancey <kbyanc@posi.net>
Cc:        arch@FreeBSD.ORG, Robert Watson <rwatson@FreeBSD.ORG>, Julian Elischer <julian@elischer.org>, Garance A Drosihn <drosih@rpi.edu>
Subject:   Re: Changes to suser() and friends
Message-ID:  <XFMail.011102112429.jhb@FreeBSD.org>
In-Reply-To: <Pine.BSF.4.21.0111011522310.40755-100000@gateway.posi.net>

next in thread | previous in thread | raw e-mail | index | archive | help

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 <jhb@FreeBSD.org> -- 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




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?XFMail.011102112429.jhb>