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: [RFA 4/4 take 2] Improved linker-debugger interface


>>>>> "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


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