This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 4/4 take 2] Improved linker-debugger interface
- From: Tom Tromey <tromey at redhat dot com>
- To: gdb-patches at sourceware dot org
- Date: Wed, 25 Jul 2012 13:36:18 -0600
- Subject: Re: [RFA 4/4 take 2] Improved linker-debugger interface
- References: <20120719110518.GE16185@redhat.com>
>>>>> "Gary" == Gary Benson <gbenson@redhat.com> writes:
Gary> +static struct probe_and_info *
Gary> +solib_event_probe_at (struct svr4_info *info, struct bp_location *loc,
Gary> + struct probe_and_info *result)
I think this would be clearer if it just returned a boolean.
Gary> + if (probe_argc == 2)
Gary> + action = NAMESPACE_RELOAD;
Gary> + else if (probe_argc < 2)
Gary> + return NAMESPACE_TABLE_INVALIDATE;
Gary> +
Gary> + return action;
Perhaps that first 'return' could be an assignment to 'action'.
Gary> +static hashval_t
Gary> +hash_namespace (const PTR p)
Use 'void *' instead of PTR. This occurs in a few spots.
PTR is obsolete.
Gary> + const struct namespace *ns = (const struct namespace *) p;
You don't need a cast here.
Gary> + const struct namespace *ns1 = (const struct namespace *) p1;
Gary> + const struct namespace *ns2 = (const struct namespace *) p2;
Here either.
Gary> + struct namespace *ns = (struct namespace *) p;
Here either.
Gary> + ns = xcalloc (sizeof (struct namespace), 1);
I think it is more normal to use XCNEW (struct namespace) here.
xcalloc with 1 element is "funny".
There's a lot of odd little macros like this, used inconsistently...
The rest of the patch looks good to me.
However, I think we should refrain from putting it in until the probes
go in to glibc. If this makes a chicken-and-egg problem somehow, we can
suggest simultaneous approval or the like.
Tom