Re: [3/18] argv_ref speedup: avoid length recomputations

[Gary V. Vaughan]

Hi Eric,

On 15 Jan 2008, at 04:35, Eric Blake wrote:

> Eric Blake <ebb9 <at> byu.net> writes:
>
> > 2007-11-29  Eric Blake  <ebb9 <at> byu.net>
> >
> >         Stage 3: cache length, rather than computing it.
> >         * src/builtin.c (define_user_macro, mkstemp_helper): Use
> >         pre-computed length.
>
> On the branch (but not head, since I haven't ported --warn-macro- 
> sequence there
> yet):

Argh!  Please don't let us get back to having code committed to the  
branch in
advance of the port to HEAD.  It is very easy to lose track of what  
needs to
go where... look what happened last time (have we even caught up with  
the patches
that were applied unilaterally on the branch a few years ago yet?).

Not wishing to be a party-pooper, but for good maintenance I think  
that it is very
important not to commit changes to the tree until they have been  
ported to all
applicable active branches.

> > @@ -407,7 +407,8 @@ free_regex (void)
> > ` 
> > -------------------------------------------------------------------------*/
> >
> > void
> > -define_user_macro (const char *name, const char *text,  
> > symbol_lookup mode)
> > +define_user_macro (const char *name, size_t len, const char *text,
> > +   symbol_lookup mode)
> > {
> >  symbol *s;
> >  char *defn = xstrdup (text ? text : "");
> > @@ -423,7 +424,6 @@ define_user_macro (const char *name, const char  
> > *text,
> symbol_lookup mode)
> >  if (macro_sequence_inuse && text)
> >    {
> >      regoff_t offset = 0;
> > -      size_t len = strlen (defn);
>
> This hunk caused a regression, because I inadvertently changed len  
> from the
> length of the macro's definition to the length of the macro name.   
> And I would
> have caught it sooner, had the test suite been testing --warn-macro- 
> sequence.

Please add a test to prevent a reregression :-)

If not, then a TODO item comprehensive enough to allow someone else to  
write the
test later is fine.

Cheers,
        Gary
--
 ())_.              Email me: address@hidden
 ( '/           Read my blog: http://blog.azazil.net
 / )=         ...and my book: http://sources.redhat.com/autobook
`(_~)_      Join my AGLOCO Network: http://www.agloco.com/r/BBBS7912

PGP.sig
Description: This is a digitally signed message part