This is the mail archive of the gdb-patches@sourceware.org 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]
Other format: [Raw text]

Re: Patch for PR 10728


Chris> +	* c-lang.h:

Empty entry; it is customary to list the new names, e.g., from an
earlier patch:

	* c-lang.h (cp_print_value_fields_rtti): Declare.

Chris> +extern void cp_initialise_statmem_stack();
Chris> +extern void cp_free_statmem_stack();

Space before open parens.  Use (void), not ().
These occur several times.

Chris> -  return val_print (val_type, value_contents_all (val),
Chris> +  {
Chris> +    int rc;
Chris> +
Chris> +    cp_initialise_statmem_stack();

initialize, with a z.

Chris> +    rc = val_print (val_type, value_contents_all (val),
Chris>  		    value_embedded_offset (val),
Chris>  		    value_address (val),
Chris>  		    stream, 0, &opts, current_language);
Chris> +
Chris> +    cp_free_statmem_stack();

There are other calls to val_print in this function.
It seems like they should all be wrapped.

If cp_free_statmem_stack must be called, then you need a cleanup.
However, does it really need to be called?  IMO it is ok to keep a
little data on this obstack in between calls as long as it is cleared at
the next call (so that we don't leak an unbounded amount over time).

It isn't clear to me from this patch or the explanation that 
cp_initialise_statmem_stack is called at every relevant entry point.
In fact it would be better not to have to do that, because auditing
this call hierarchy is tricky.

E.g., m2-valprint.c has a call to cp_print_value_fields.  Does that
matter?

I didn't read the old code in depth; is there a way to make this
self-initializing?

Chris> +/* initialise to empty the static member stack */

Comments should start with a capital and end with a period & two
spaces.  See the GNU Coding Standards.

Chris> +      void * statmem_obstack_top = NULL;

No space after the "*".

Chris> -      first_dont_print
Chris> -	= (CORE_ADDR *) obstack_base (&dont_print_statmem_obstack);
Chris> -      i = (CORE_ADDR *) obstack_next_free (&dont_print_statmem_obstack)
Chris> -	- first_dont_print;
Chris> +      first_dont_print =
Chris> +	(CORE_ADDR *)obstack_base (&dont_print_statmem_obstack);
Chris> +      i	=
Chris> +	obstack_object_size (&dont_print_statmem_obstack) / sizeof(CORE_ADDR);
 
The formatting is weird here; the first statement doesn't appear to have
changed, but has gratuitous formatting changes, and the second statement
has a tab before the "=".

Chris> +send_gdb "print b\n"
Chris> +
Chris> +gdb_expect {
Chris> +    -re "same as static member" {
Chris> +	pass "print b"
Chris> +    }
Chris> +    timeout { fail "(timeout) print b" }
Chris> +}

Don't use sent + gdb_expect.  Use gdb_test.

Tom


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