Re: [RFA 4/4 take 2] Improved linker-debugger interface

[Gary Benson <gbenson@redhat.com>]

Tom Tromey wrote:
> >>>>> "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...

Attached is an updated version of this patch with these fixes.

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

Probes are in glibc as of Friday:

  http://sourceware.org/ml/libc-alpha/2012-07/msg00557.html

Cheers,
Gary

-- 
http://gbenson.net/