This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC 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: default plugin directory?


Hello Basile,

this is not an approval, merely writing what I see:

* Basile STARYNKEVITCH wrote on Fri, Jun 19, 2009 at 08:04:50AM CEST:
> --- gcc/plugin.c	(revision 148688)
> +++ gcc/plugin.c	(working copy)
> @@ -590,6 +590,45 @@ try_init_one_plugin (struct plugin_name_args *plug
>    PTR_UNION_TYPE (plugin_init_func) plugin_init_union;
>  
>    dl_handle = dlopen (plugin->full_name, RTLD_NOW);
> +
> + /* If we cannot find the plugin by its full name, try to get it in
> +    the default directory: if passing -fplugin=foo, look there for
> +    foo.so */

> +      /* construct the plugin path, possibly adding a .so suffix if it
> +	 does not appear in the full name */

Wouldn't that be useful also before the dlopen above?  I'm not sure,
probably it doesn't matter.

> +      if (relocated_prefix)
> +	pluginpath
> +	  = concat (relocated_prefix,
> +		    "/", /* maybe use DIR_SEPARATOR */
> +		    plugin->full_name,
> +		    strstr(plugin->full_name, ".so") ? "" : ".so",

This fould falsely not add .so to plugins that have .so somewhere in the
middle of their name, but not at the end.

> +		    NULL);
> +      if (pluginpath)
> +	{
> +	  dl_handle = dlopen (pluginpath, RTLD_NOW);
> +	  free(pluginpath);
> +	}
> +    }
> +
>    if (!dl_handle)


> --- gcc/Makefile.in	(revision 148688)
> +++ gcc/Makefile.in	(working copy)

> @@ -2517,7 +2519,13 @@ passes.o : passes.c $(CONFIG_H) $(SYSTEM_H) corety
>  
>  plugin.o : plugin.c $(PLUGIN_H) $(CONFIG_H) $(SYSTEM_H) coretypes.h \
>     $(TOPLEV_H) $(TREE_H) $(TREE_PASS_H) intl.h $(PLUGIN_VERSION_H) $(GGC_H)
> +	$(COMPILER) $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS)  \
> +         -DPLUGIN_DEFAULT_DIR=\"$(plugins_default_dir)\" \
> +         -DSTANDARD_PREFIX=\"$(prefix)\" \
> +         -DSTANDARD_EXEC_PREFIX=\"$(libdir)/gcc/\" \
> +         -c $(srcdir)/plugin.c $(OUTPUT_OPTION)

I think you should let plugin.o depend on Makefile as it encodes
$(prefix) and other variables set by the Makefile, so that it is rebuilt
if the user changes it with a new configure command.

You haven't stated how you have tested the patch.  (Well, you have
stated that you hadn't tested the first version of your patch, but
my guess is that won't help you get approval from a maintainer.)

Ideally, GCC would have an installcheck rule which one could use to
exercise this code on an installed compiler, relocated or not, with
plugin names that exist, or not, contain .so in the name, or not.

I suppose it would be a good cleanup patch to kill this static
relocated_prefix variable from your patch and from incpath.c and
make it a global variable instead, computed once, or maybe make it
a function that computes it lazily, to at least avoid the code
duplication.

Cheers,
Ralf


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