Re: ntsec patch #4: passwd and group

["Pierre A. Humblet" <Pierre dot Humblet at ieee dot org>]

Corinna Vinschen wrote:
> 
> Some questions and comments:
> 
> >
> >       * security.h: [...]. Undeclare internal_getpwent.
>                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^
>                              You didn't.
Sorry, looks like I messed up my versions. It was done at some point.

> > -/* FIXME: should be static but this is called in uinfo_init outside this
> > -   file */
> > -void
> > +static void
> >  read_etc_group ()
> 
> Do I miss something?  I don't see that in this patch.

What do you mean? Your comment is in the middle of a patch.
If you are talking about the FIXME, it must be ancient. The problem
was gone before I did anything.

> > +
> > +      /* Complete /etc/group in memory if needed */
> > +      if (!getgrgid32 (myself->gid))
> 
> ?!? How is that supposed to work?  We're in group_state==initializing,
> therefore in getgrgid32(), read_etc_group() is called.  Isn't that
> somewhat dangerous?

Yes, I was hesitant to do it but never saw any problem. 
group_state.set_last_modified () has just been called, but even
if the file had been modified again I think we would be OK.
That will soon go away anyway, see below. 

> Didn't you propose to get rid of the LookupAccountSidA() calls?

Yes, I did. It wasn't clear to me that you agreed. 
 
> Ahem, I thought we agreed that we don't call external functions from
> inside Cygwin?  Never mind, there are still some of them which we have
> to eliminate, anyway.

I didn't know about that policy but it suits me fine. As we discussed,
internal calls to passwd/group functions should never reread the files,
so new entry points are called for. I was going to do that in a second
step, it wasn't a goal when I started.

> > +  if ((pw = getpwuid32 (uid)))
> 
> Same here.  Somehow it's a step in the wrong direction...
See above. The goal is to avoid (indirectly) calling WaitForSingleObject
for each line in the passwd file. 

How do you want to proceed? Apply this patch and undeclare internal_getpwent,
remove LookupAccountSidA(), apply your "I'd better like" and introduce
internal lookup functions in a few days, or prepare a single all-encompassing
patch in a few days?

Pierre