This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [RFA]: Massive C++ fixes and enhancements


On Thu, 10 Aug 2000, Kevin Buettner wrote:

> Daniel,
> 
> My comments on your patch:
> 
> 1) I notice that you're still getting rid of the do { ...  } while (0)
>    in SYMBOL_INIT_DEMANGLED_NAME...   I realize that you don't like this
>    construct, but if someone decides to use SYMBOL_INIT_DEMANGLED_NAME
>    as part of an if-then-else construct somewhere, you'll save them a
>    lot of grief.  (If you really, really don't like this, then just
>    make it a function.)

I plan on making it a function, actually.
THe reason for getting rid of it was because I hand edited the patch i
sent out last time, and forgot to actually undo the change in the source.


> 
> 2) Watch your indentation.  E.g, the following hunk from 
>    decode_line_1() doesn't look right to me:
> 
> --- 2577,2602 ----
>       }
>     else
>       is_quote_enclosed = 0;
> + tryagain:
>     for (; *p; p++)
>       {
> 	if (p[0] == '<')
> 	{
> 	  char *temp_end = find_template_name_end (p);
> ! 	  if (!temp_end && !triedonce)
> ! 	  {
> ! 		  triedonce=1;
> ! 		  is_quote_enclosed=1;
> ! 		  p=*argptr;
> ! 		  if (has_comma)
> ! 			  *ii=',';
> ! 		  s=NULL;
> ! 		  goto tryagain;
> ! 	   }
> ! 	  else if (!temp_end && triedonce)
> ! 	  {
> 	    error ("malformed template specification in command");
> + 	  }
> 	  p = temp_end;
> 	}
> 	/* Check for the end of the first half of the linespec.  End of line,
> 
> 
> Here's how (a portion of it) ought to look:
> 
> ! 	  if (!temp_end && !triedonce)
> ! 	    {
> ! 	      triedonce=1;
> ! 	      is_quote_enclosed=1;
> ! 	      p=*argptr;
> ! 	      if (has_comma)
> ! 	        *ii=',';
> ! 	      s=NULL;
> ! 	      goto tryagain;
> ! 	    }
> ! 	  else if (!temp_end && triedonce)
> ! 	    {
> 	      error ("malformed template specification in command");
> + 	    }

It's actually is the wrong indentation, i had forgotten to reindent that
part in emacs, i had edited it in vi for various reasons.

It's now correctly indented in the source, and i got rid of the {} around
theerror.

> 
> Also, note that GNU coding standard doesn't require the braces around
> the call to error().  I.e, you could've left that part of the original
> code alone.

Fixed.
> 
> 3) You might want to get into the habit of having separate source
>    and build trees.
> 

I have about 6 build trees, and 3 source trees right now.

I just built checked it out and built it in the same dir because i had
other changes in the tree i normally use that i didn't want to submit yet.


--Dan



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]