RE: [RFA] p-typeprint.c, move pointer use to after null-check.

["Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : mardi 8 mars 2011 06:10
> À : Michael Snyder
> Cc : gdb-patches@sourceware.org; Pierre Muller
> Objet : Re: [RFA] p-typeprint.c, move pointer use to after null-check.
> 
> > If it's worth checking for null...
> >
> > OK?
> >
> 
> > 2011-03-01  Michael Snyder  <msnyder@vmware.com>
> >
> > 	* p-typeprint.c (pascal_type_print_method_args): Don't use
> > 	pointer until after null-check.
> I think I get the drift of the code, and ISTM that the check for NULL
> might be misleading. I think that "physname" can never be null, by
> virtue of how it's called. What I would do is just remove the check
> against NULL (we can add a gdb_assert at the same time, which would
> force us to declare the is_constructor/is_destructor variables without
> initial value - no big deal).
> 
> Pierre?

  No, I think that the code relies on the fact that physname
is never null, but that the constructor or destructor could have no
parameters, in which case  physname would point to \0
after the '+= 6'.
  Parameterless pascal procedure/functions
do not require opening/closing braces like C does.
  So I still think that the current code is correct.
 So to answer to Michael, with my apologies for the delay,
I do think that your patch is not correct and should
not be applied as is.
 
> A few remarks:
> 
> The function could (should?) be made static, unless I grep'ed wrong
  Should indeed...
  
 
> > +      if (is_constructor || is_destructor)
> > +	{
> > +	  physname += 6;
> > +	}
> 
> Useless extra curly braces...
 OK, while correcting these, I also saw that:  

Is this correct:
   if (physname && (*physname != 0))
or is:
   if (physname && *physname != 0)

better?
or should I use:
   if (physname && *physname != '\0')


Pierre Muller
GDB pascal language maintainer