Re: bin/41700: :tr modifier for NetBSD make [patch]

[David Laight <david%l8s.co.uk@localhost>]

The following reply was made to PR bin/41700; it has been noted by GNATS.

From: David Laight <david%l8s.co.uk@localhost>
To: gnats-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: bin/41700: :tr modifier for NetBSD make [patch]
Date: Sat, 11 Jul 2009 14:13:00 +0100

 On Sat, Jul 11, 2009 at 11:20:00AM +0000, cheusov%tut.by@localhost wrote:
 > >Number:         41700
 > >Category:       bin
 > >Synopsis:       :tr modifier for NetBSD make [patch]
 ...
 > Now I propose a patch.
 
 Some comments about the code....
 (NB not about whether this is a good idea)
 
 > +static char *
 > +VarTranslate(char *str, char *chars, char *repl)
 > +{
 > +   Buffer  buf;
 > +   int     table [UCHAR_MAX+1];
 > +   int     r;
 > +   int     c;
 > +
 > +   /* initialization of translation table */
 > +   memset (table, -1, sizeof (table));
 
 initialize table[n] = n here, table can then be char.
 
 > +   for (; *chars; ++chars, ++repl){
 > +       assert (*repl != 0);
 
 Don't assert! I can't remember what tr(1) does, but terminating
 the action wouldn't be too bad.
 
 > +       table [(unsigned char) *chars] = (unsigned char) *repl;
 
 You need to support ranges!
 
 > +   }
 > +   assert (*repl == 0);
 > +
 > +   /* translation */
 
 Don't use 'buf', just tgt = malloc(strlen(str) + 1)
 
 > +   Buf_Init(&buf, 0);
 > +   for (; *str ; ++str) {
 > +       c = (unsigned char) *str;
 > +       r = table [c];
 > +       if (r == -1)
 > +       Buf_AddByte(&buf, (Byte)c);
 > +       else
 > +       Buf_AddByte(&buf, (Byte)r);
 > +   }
 > +   Buf_AddByte(&buf, (Byte)'\0');
 In any case Buf_Addbyte guarantees \0 terminated
 
 > +   str = (char *)Buf_GetAll(&buf, NULL);
 > +   Buf_Destroy(&buf, FALSE);
 > +   return str;
 > +}
 > +
 > +/*-
 > + *-----------------------------------------------------------------------
 >   * VarChangeCase --
 >   *      Change the string to all uppercase or all lowercase
 >   *
 > @@ -2655,7 +2707,47 @@
 >      case 't':
 >          {
 >              cp = tstr + 1;  /* make sure it is set */
 > -            if (tstr[1] != endc && tstr[1] != ':') {
 > +            if (tstr[1] == 'r') {
 > +                char *chars, *repl;
 > +                int chars_len, repl_len;
 > +
 > +                delim = tstr[2];
 > +                tstr += 3;
 > +
 > +                cp = tstr;
 > +                chars = VarGetPattern(
 > +                    ctxt, &parsestate, errnum,
 > +                    &cp, delim, NULL,
 > +                    &chars_len,
 > +                    NULL);
 > +                if (!chars)
 > +                    goto cleanup;
 > +
 > +                repl = VarGetPattern(
 > +                    ctxt, &parsestate, errnum,
 > +                    &cp, delim, NULL,
 > +                    &repl_len,
 > +                    NULL);
 > +                if (!repl)
 > +                    goto cleanup;
 > +
 > +                delim = '\0';
 
 No need to do the check below twice!
 > +                if (strlen (chars) != strlen (repl)){
 > +                    free(chars);
 > +                    free(repl);
 > +
 > +                    Error("Bad arguments for :tr modifier for variable %s",
 > +                          v->name);
 > +                    goto cleanup;
 > +                }
 > +
 > +                termc = *cp;
 > +                newStr = VarTranslate(nstr, chars, repl);
 > +
 > +                free(chars);
 > +                free(repl);
 > +            }else if (tstr[1] != endc && tstr[1] != ':') {
 >                  if (tstr[1] == 's') {
 >                      /*
 >                       * Use the char (if any) at tstr[2]
 > 
 
 
        David
 
 -- 
 David Laight: david%l8s.co.uk@localhost