Re: PATCH: Improve plugin error handling

[Alan Modra <amodra at gmail dot com>]

On Fri, Nov 23, 2012 at 08:13:14AM -0800, H.J. Lu wrote:
> +#ifdef HAVE_DLFCN_H
> +static const char *
> +dl_error (const char *plugin ATTRIBUTE_UNUSED)
> +{
> +  return dlerror ();
> +}
> +#else
> +static const char *
> +dl_error (const * char plugin)
> +{
> +  return plugin;
> +}
> +#endif
> +

I think the above would be better without the "plugin" arg, with the
!HAVE_DLFCN_H case returning "".

> @@ -188,7 +202,7 @@ plugin_opt_plugin (const char *plugin)
>    newplug->name = plugin;
>    newplug->dlhandle = dlopen (plugin, RTLD_NOW);
>    if (!newplug->dlhandle)
> -    return set_plugin_error (plugin);
> +    einfo (_("%P%F: error loading plugin: %s\n"), dl_error (plugin));

I think this message should still print the plugin name.  ie.
    einfo (_("%P%F: %s: error loading plugin: %s\n"), plugin, dl_error ());

> @@ -806,13 +819,15 @@ plugin_load_plugins (void)
>        if (!onloadfn)
>  	onloadfn = (ld_plugin_onload) dlsym (curplug->dlhandle, "_onload");
>        if (!onloadfn)
> -	return set_plugin_error (curplug->name);
> +	einfo (_("%P%F: error loading plugin: %s\n"),
> +	       dl_error (curplug->name));

Similarly here.

>        set_tv_plugin_args (curplug, &my_tv[tv_header_size]);
>        called_plugin = curplug;
>        rv = (*onloadfn) (my_tv);
>        called_plugin = NULL;
>        if (rv != LDPS_OK)
> -	return set_plugin_error (curplug->name);
> +	einfo (_("%P%F: %s: error loading plugin: %d\n"),
> +	       curplug->name, rv);

You've successfully loaded the plugin here, but hit some runtime error.
Perhaps just s/error loading plugin/plugin error/.

OK with those changes, and of course altering the testsuite to suit.

-- 
Alan Modra
Australia Development Lab, IBM