Mailing-List: contact cygwin-developers-help AT cygwin DOT com; run by ezmlm
List-Subscribe: <mailto:cygwin-developers-subscribe AT cygwin DOT com>
List-Archive: <http://sources.redhat.com/ml/cygwin-developers/>
List-Post: <mailto:cygwin-developers AT cygwin DOT com>
List-Help: <mailto:cygwin-developers-help AT cygwin DOT com>, <http://sources.redhat.com/ml/#faqs>
Sender: cygwin-developers-owner AT cygwin DOT com
Delivered-To: mailing list cygwin-developers AT cygwin DOT com
Date: Sat, 25 Jan 2003 23:55:10 -0500
From: Christopher Faylor <cgf AT redhat DOT com>
To: cygwin-developers AT cygwin DOT com
Subject: Re: passwd/group parsing
Message-ID: <20030126045510.GA1122@redhat.com>
Reply-To: cygwin-developers AT cygwin DOT com
Mail-Followup-To: cygwin-developers AT cygwin DOT com
References: <3 DOT 0 DOT 5 DOT 32 DOT 20030125204634 DOT 00802ad0 AT mail DOT attbi DOT com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3.0.5.32.20030125204634.00802ad0@mail.attbi.com>
User-Agent: Mutt/1.5.1i

On Sat, Jan 25, 2003 at 08:46:34PM -0500, Pierre A. Humblet wrote:
>Chris,
>
>I had a quick look at your changes and have a few comments:
>- soon the uid/gid will be __uid32_t and the code should support it. 
>  It is legal for next_int () to return negative values, even -1.
>  That's why the original code was using another error detection
>  method.

Ok.  I'll change this to use the appropriate types.

>- It looks like fields such as pw_gecos, pw_dir, etc.. could be set
>  to NULL when a line is incomplete. That is a new behavior, not 
>  expected by internal code nor by programs such as login (I only 
>  checked that one).

I should have checked too.  I just verified that UNIX puts real values
in these fields.

>  It comes from the code in next_str. By the way, why do you add 
>  two \0 in "search" used by strpbrk? The second never has any effect.

pwdgrp::next_str (char c)
{ 
  char search[] = ":\n\0\0";
  search[2] = c;
              ^
          an additional character to parse for

>- 99.99% of /etc/group lines at cygwin installations have an empty
>  last field. That's why when that was detected (dp == NULL with your 
>  new code, *dp == 0 with the old or new) gr_mem was set to null_ptr
>  rather than pointing dp to a static empty string, scanning it twice 
>  and calling calloc.

Ok.  I like optimizations like that.  Sorry I missed it.  I actually
meant to go back and do a final sanity check before I checked everything
in to make sure that the new code worked the same as the old and... I
forgot...

>  I believe the old code was also setting gr_mem to a non-NULL legal 
>  value in the unlikely event where calloc failed.

Hmm.  From the linux manpage, it looks like maybe it should be setting ENOMEM
however there's no need to change the behavior, so I've put it back.  It
actually simplifies the code a little.

>  Incidentally setting namearray[i] = NULL is useless, calloc has
>  already done it (that may be legacy code).

Yes, that was legacy code.  I removed it.

I don't remember if I mentioned this before, but merging the code
between passwd.cc and grp.cc is one thing that I have wanted to do for
years.  Corinna got things partway there at my request.  I'm trying to
get it all of the way now.  Using common routines for parsing these
files, which use common formats makes a lot of sense, IMO.

Btw, I also took your suggestion and made load not return a boolean.

cgf