On Monday 15 March 2010 21:00:38 Pedro Alves wrote:

> On Sunday 14 March 2010 08:58:36, Vladimir Prus wrote:
> > +  name = argv[0];
> > +  if (name[0] != '$')
> > +    error (_("Name of trace variable should start with '$'"));
> > +  ++name;
> > +
> > +  expr = parse_expression (argv[0]);
> > +  back_to = make_cleanup (xfree, expr);
> > +
> > +  if (expr->nelts == 3 && expr->elts[0].opcode == OP_INTERNALVAR)
> > +    {
> > +      struct internalvar *intvar = expr->elts[1].internalvar;
> > +      if (intvar)
> > +       name = internalvar_name (intvar);
> > +    }
> > +
> > +  if (!name || *name == '\0')
> > +    error (_("Invalid name of trace variable"));
> 
> Waitaminute.  Is there a merge error here?
> 
> Repeating the snippet:
> 
> > +  name = argv[0];
> > +  if (name[0] != '$')
> > +    error (_("Name of trace variable should start with '$'"));
> > +  ++name;
> > +
> 
> I think this whole bit above shouldn't be here.

Yes, it's a merge error.

> > +  expr = parse_expression (argv[0]);
> > +  back_to = make_cleanup (xfree, expr);
> > +
> > +  if (expr->nelts == 3 && expr->elts[0].opcode == OP_INTERNALVAR)
> > +    {
> > +      struct internalvar *intvar = expr->elts[1].internalvar;
> > +      if (intvar)
> > +       name = internalvar_name (intvar);  <<<<<<<< (1)
> > +    }
> > +
> > +  if (!name || *name == '\0')
> > +    error (_("Invalid name of trace variable"));
> 
> Otherwise, it looks like you can get here with an invalid name, if
> the expression did parse sucessfully, but (1) wasn't reached at all.

At least after removing the unnecessary bit, if (!) is not executed,
name will be NULL -- which seems right behaviour.


Thanks,

-- 
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722