This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: PATCH: Improve plugin error handling
- From: Alan Modra <amodra at gmail dot com>
- To: "H.J. Lu" <hjl dot tools at gmail dot com>
- Cc: binutils at sourceware dot org
- Date: Mon, 3 Dec 2012 13:58:51 +1030
- Subject: Re: PATCH: Improve plugin error handling
- References: <20121123161314.GA8902@gmail.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