From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Elizabeth Chastain <chastain@cygnus.com>
To: dberlin@redhat.com, gdb-patches@sources.redhat.com
Subject: Re: [PATCH] New C++ abstraction patch
Date: Tue, 20 Feb 2001 17:35:00 -0000
Message-id: <200102210135.RAA21426@bosch.cygnus.com>
X-SW-Source: 2001-02/msg00409.html

Hi Daniel,

I have some comments on your patch.

Michael

===

+ gnu-v3-abi.o: gnu-v3-abi.c cp-abi.h
+ 
+ gnu-v2-abi.o: gnu-v2-abi.c cp-abi.h
+ 

gnu-v3-abi.o and gnu-v2-abi.o also depend on
$(defs_h) gdb_regex.h gdb_string.h.



+ int gnuv2_vtable_prefix_p (const char * name)
+ {
+   return (((name)[0] == '_' 
+ 	   && (((name)[1] == 'V' && (name)[2] == 'T') 
+ 	       ||  ((name)[1] == 'v' && (name)[2] == 't'))
+ 	   && is_cplus_marker ((name)[3])) || 
+ 	  ((name)[0] == '_' && (name)[1] == '_' 
+ 	   && (name)[2] == 'v' && (name)[3] == 't' && (name)[4] == '_'));
+ }

Would you consider writing this as:

  int gnuv2_vtable_prefix_p (const char * name)
  {
    if (strncmp (name, "_VT", 3) == 0 && is_cplus_marker (name[3]))
      return 1;
    if (strncmp (name, "_vt", 3) == 0 && is_cplus_marker (name[3]))
      return 1;
    if (strncmp (name, "__vt_", 5) == 0)
      return 1;
    return 0;
  }

Or if that is not to your taste, something that is more readable than
the former macro?

(This is a style issue rather than a correctness issue so I would
accept whatever you decide to write, including the original).



+ //struct cp_abi_ops current_cp_abi;

This would be the first use of // comments in gdb.  There are some
instances of '//', but they are already inside comments, or they are
part of /* ... *//* ... */ constructions.



+ /* FIXME: Stop doing the re_comp's everytime. Use the other regex api
+    and compile the patterns ahead of time */

One of the other uses of re_comp/re_exec is in solib_add, which calls
symbol_file_add in a loop.  I think you're in danger of overwriting the
single global re_comp buffer.

There are other uses in target-specific solib loaders, like irix and hpux.

I like regular expressions but I think you need regcomp/regexec/regerror
rather than re_comp/re_exec.



+ int gnuv3_destructor_prefix_p (const char * name)
+ {
+   re_comp ("[^0-9]D[1-3]Ev");
+   return re_exec (name) != 0;
+ }

libiberty/cp-demangle.c says that dtor-names are D0 D1 D2, not D1 D2 D3.

What happens if the target program has a method named TOORAD2Evade?
I think your r.e. would match that mangled name.



+ int gnuv3_vtable_prefix_p (const char * name)
+ {
+   return strncmp (name, "_ZTV", 4) != 0;
+ }

This strncmp() != 0 should be strcmp() == 0.  (silly strncmp)