patchfixup GECOS handling

> On Mon, 2005-04-18 at 12:36 +0200, Martin Schlemmer wrote:
> > realgecos[strchr(realgecos, ',') - realgecos] = '\0';
> 
> Er, *strchr(realgecos, ',') = 0; surely? Even if the compiler is clever
> enough to optimise out the gratuitous addition and subtraction, that's
> no real excuse for it.
> 

>         if (!pw)
>                 die("You don't exist. Go away!");
>         realgecos = pw->pw_gecos;
> +       /* The name is seperated from the room no., tel no, etc via [,;] */
> +       if (strchr(realgecos, ','))
> +               *strchr(realgecos, ',') = 0;
> +       else if (strchr(realgecos, ';'))
> +               *strchr(realgecos, ';') = 0;
>         len = strlen(pw->pw_name);
>         memcpy(realemail, pw->pw_name, len);
>         realemail[len] = '@';

> On Fri, 2005-04-22 at 16:23 +0200, Martin Schlemmer wrote:
> > Hi,
> > 
> > This still applies - any reason for not doing this?
> 
> Seems like this will break on certain kinds of data.  See below.
> 
> >         if (!pw)
> >                 die("You don't exist. Go away!");
> >         realgecos = pw->pw_gecos;
> > +       /* The name is seperated from the room no., tel no, etc via [,;] */
> > +       if (strchr(realgecos, ','))
> > +               *strchr(realgecos, ',') = 0;
> > +       else if (strchr(realgecos, ';'))
> > +               *strchr(realgecos, ';') = 0;
> >         len = strlen(pw->pw_name);
> >         memcpy(realemail, pw->pw_name, len);
> >         realemail[len] = '@';
> 
> Suppose that the GECOS field is:
> 
> Hayes, Kyle; Room 42; 424-424-4242; foo bar baz...
> 
> You'll search for the first comma, find it, truncate my name to "Hayes",
> and continue.
> 
> I have seen this kind of GECOS in larger environments where the
> individual users are not the ones that administrate their machines.
> Using the LastName, FirstName style of name is not rare. 
> 

> Dear diary, on Fri, Apr 22, 2005 at 06:58:25PM CEST, I got a letter
> where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> > Meaning, if they use a ',' in one of the fields (and it is a linux
> > system with the chfn most probably from the shadow package), then they
> > are looking for trouble.  The only reason I added the ';' was because
> > somebody said whatever OS used it instead of a ','.
> 
> What about just swapping the two tests so that ; is cut off and , only
> when no ; is around?
> 

> Dear diary, on Fri, Apr 22, 2005 at 06:58:25PM CEST, I got a letter
> where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> > Meaning, if they use a ',' in one of the fields (and it is a linux
> > system with the chfn most probably from the shadow package), then they
> > are looking for trouble.  The only reason I added the ';' was because
> > somebody said whatever OS used it instead of a ','.
> 
> What about just swapping the two tests so that ; is cut off and , only
> when no ; is around?

> On Fri, 2005-04-22 at 19:18 +0200, Petr Baudis wrote:
> > Dear diary, on Fri, Apr 22, 2005 at 06:58:25PM CEST, I got a letter
> > where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> > > Meaning, if they use a ',' in one of the fields (and it is a linux
> > > system with the chfn most probably from the shadow package), then they
> > > are looking for trouble.  The only reason I added the ';' was because
> > > somebody said whatever OS used it instead of a ','.
> > 
> > What about just swapping the two tests so that ; is cut off and , only
> > when no ; is around?
> 
> Even nicer.  I like it.  Very clean!
> 

> ----
> (not signed off, etc, as just for comments)
> 
> Index: commit-tree.c
> ===================================================================
> --- 5f61aecb06c2f2579bbb5951b1b53e0dedc434eb/commit-tree.c  (mode:100644 sha1:c0b07f89286c3f6cceae8122b4c3142c8efaf8e1)
> +++ uncommitted/commit-tree.c  (mode:100644)
> @@ -96,21 +96,6 @@
>                 if (!c)
>                         break;
>         }
> -
> -       /*
> -        * Go back, and remove crud from the end: some people
> -        * have commas etc in their gecos field
> -        */
> -       dst--;
> -       while (--dst >= p) {
> -               unsigned char c = *dst;
> -               switch (c) {
> -               case ',': case ';': case '.':
> -                       *dst = 0;
> -                       continue;
> -               }
> -               break;
> -       }
>  }
> 
>  static const char *month_names[] = {
> @@ -311,6 +296,17 @@
>         if (!pw)
>                 die("You don't exist. Go away!");
>         realgecos = pw->pw_gecos;
> +       /*
> +        * The GECOS fields are seperated via ',' on Linux, FreeBSD, etc,
> +        * and ';' on AIX.
> +        */
> +#if defined(__aix__)
> +       if (strchr(realgecos, ';'))
> +               *strchr(realgecos, ';') = 0;
> +#else
> +       if (strchr(realgecos, ','))
> +               *strchr(realgecos, ',') = 0;
> +#endif
>         len = strlen(pw->pw_name);
>         memcpy(realemail, pw->pw_name, len);
>         realemail[len] = '@';
> 
> 

> @@ -311,6 +296,17 @@
>         if (!pw)
>                 die("You don't exist. Go away!");
>         realgecos = pw->pw_gecos;
> +       /*
> +        * The GECOS fields are seperated via ',' on Linux, FreeBSD, etc,
> +        * and ';' on AIX.
> +        */
> +#if defined(__aix__)
> +       if (strchr(realgecos, ';'))
> +               *strchr(realgecos, ';') = 0;
> +#else
> +       if (strchr(realgecos, ','))
> +               *strchr(realgecos, ',') = 0;
> +#endif
>         len = strlen(pw->pw_name);
>         memcpy(realemail, pw->pw_name, len);
>         realemail[len] = '@';

> Dear diary, on Fri, Apr 22, 2005 at 09:06:43PM CEST, I got a letter
> where Martin Schlemmer <azarah@nosferatu.za.org> told me that...
> > @@ -311,6 +296,17 @@
> >         if (!pw)
> >                 die("You don't exist. Go away!");
> >         realgecos = pw->pw_gecos;
> > +       /*
> > +        * The GECOS fields are seperated via ',' on Linux, FreeBSD, etc,
> > +        * and ';' on AIX.
> > +        */
> > +#if defined(__aix__)
> > +       if (strchr(realgecos, ';'))
> > +               *strchr(realgecos, ';') = 0;
> > +#else
> > +       if (strchr(realgecos, ','))
> > +               *strchr(realgecos, ',') = 0;
> > +#endif
> >         len = strlen(pw->pw_name);
> >         memcpy(realemail, pw->pw_name, len);
> >         realemail[len] = '@';
> 
> I'm confused, what does this has to do with AIX? Do we even have / can
> expect to have any major AIX users?
> 

> I'm not too happy with this, I have to say. It seems it won't do always
> the right thing anyway. I would still favour the approach when you cut
> off everything after ';', and everything after ',' if no ';' is found.
> Seems simplest, safest, etc.
> 
> Tell me about anyone who has a semicolon in his realname.
> 

> On Fri, 2005-04-22 at 09:16 -0700, Kyle Hayes wrote:
> > Suppose that the GECOS field is:
> > 
> > Hayes, Kyle; Room 42; 424-424-4242; foo bar baz...
> > 
> > You'll search for the first comma, find it, truncate my name to "Hayes",
> > and continue.
> > 
> > I have seen this kind of GECOS in larger environments where the
> > individual users are not the ones that administrate their machines.
> > Using the LastName, FirstName style of name is not rare. 
> > 
> 
> What OS?  With Linux at least, this is what chfn's manpage say:

> ----
>        The only restriction placed on the contents of the fields is that no control characters may  be  present,
>        nor  any  of  comma, colon, or equal sign. The other field does not have this restriction, and is used to
>        store accounting information used by other applications.
> ----
> 
> Meaning, if they use a ',' in one of the fields (and it is a linux
> system with the chfn most probably from the shadow package), then they
> are looking for trouble.  The only reason I added the ';' was because
> somebody said whatever OS used it instead of a ','.

> if(comma)
> 	if(semi)
> 		/* lastname, firstname; room #; phone # format */
> 		*semi  = 0;
> 	else
> 		*comma = 0;
> else if(semi)
> 	*semi = 0;

> On Fri, Apr 22, 2005 at 09:16:39AM -0700, Kyle Hayes wrote:
> > if(comma)
> > 	if(semi)
> > 		/* lastname, firstname; room #; phone # format */
> > 		*semi  = 0;
> > 	else
> > 		*comma = 0;
> > else if(semi)
> > 	*semi = 0;
> 
> That's a really complicated way of writing
> 
> if(semi) *semi = 0;
> else if(comma) *comma = 0;
> 
> (The two code fragments are precisely identical.  Mmmm, strength
> reduction.)