Re: revised patch for glib compilation
- From: Gary V.Vaughan <gvv techie com>
- To: Tim Janik <timj gtk org>
- Cc: Gtk+ Developers <gtk-devel-list gnome org>
- Subject: Re: revised patch for glib compilation
- Date: Tue, 20 Mar 2001 01:31:44 +0000
Hi Tim,
Thanks for the feedback. I have addressed all but one of your comments in
the attached files -- if you could help me to understand how to resolve the
remaining one, I'll do that and attach the result to bugzilla. [[Aside:
assuming it may take a number of months before this stuff arrives in CVS, I'm
guessing that directly attaching the files is preferable to a large
bitrotting patch file?]]
On Monday 19 March 2001 4:14 pm, Tim Janik wrote:
> On Sat, 24 Feb 2001, Gary V. Vaughan wrote:
> >
> > Let me know what you think and if you approve I will incorporate any
> > suggestions you have, clean the whole thing up and resubmit.
>
> ok, comments on the patch are below. i think we can really go that route,
> however we need to clear up some locking and module naming issues in your
> code, and definitely need to retain reference counts on the GModule side.
>
> > http://bugzilla.gnome.org/show_bug.cgi?id=51583
> > +
> > + Deprecati[[ng]] gmodule is a smashing idea, though I'd like to see a
> > + source compatibility wrapper modelled after this patch for the next
> > + stable release or two for the sake of projects that already use the
> > + gmodule API.
>
> we will definitely _not_ deprecate GModule as a whole. just changing
> the backend is not a valid reason to throw away the GModule API, and
> it'd also be an additional burden on people to demand they completely
> switch to libtool which doesn't work with GLib threads out of the box.
Agreed.
> s/INI/INIT/ would improve readability ;)
If only they were all so easy ;-D
> > /* --- prototypes --- */
> > static inline gboolean g_module_init_once (void);
> > static inline void g_module_set_error (const gchar *error);
> > static const gchar *g_module_get_error (void);
> > static void g_module_mutex_lock (void);
> > static void g_module_mutex_unlock (void);
> >
> > /* --- variables --- */
> > G_LOCK_DEFINE_STATIC (GModule);
>
> this is a recursive mutex in our current implementation, because modules
> might want to load other modules from their g_module_check_init()
> functions.
This is the outstanding issue that I don't understand. I can see the need
for a recursive lock, but I have no idea of how to express that in terms of
the macros in CVS HEAD gthread.h. Is there an example of recursive G_LOCKing
on the `net I can look at?
> > const char *g_log_domain_gmodule = "GModule";
>
> this should be taken care of by -DG_LOG_DOMAIN=\"GModule\" now.
Variable removed. Thanks.
> /* please use this style for
> * multiline comments
> */
Sorry. Lapse of concentration. Done.
> > if (++initialized == 1)
> > {
> > gint errors = 0; /* Number of errors from libltdl calls. */
>
> is it really required to increment initialized everytime this function is
> called? ;)
Nope, I was trying to save a couple of lines of code. Dumb, I know. Also
fixed.
> > /* Make sure libltdl is using our memory managment API. */
> > lt_dlmalloc = (lt_ptr(*)(size_t)) g_malloc;
> > lt_dlfree = (void (*)(lt_ptr)) g_free;
>
> is this required? it's safe to use malloc/free besides the glib API,
> as long as things aren't mixed ala malloc/g_free or g_malloc/free.
It is not a requirement, but it allows G_MEM_PROFILING to see what libltdl
did with your memory (for example). If you find it onerous, I can delete
those lines.
> > /* Initialise libltdl module search directory list. */
> > if (path != NULL)
> > errors = lt_dladdsearchdir (path);
>
> can this be reset later? some module loadig APIs only want a specific
> search path for their modules. i think in this point, our API still has
> issues, as for gtk modules, another search path is required as e.g. for
> loading application specific plugins. using a common search path might not
> always be desirable.
Yep. lt_dlsetsearchdir() will overwrite the current search path, and
additional calls to lt_dladdsearchdir() will add additional directories.
Beyond that, the caller can g_module_open a full module pathname to prevent
path searching.
> multiline warnings shouldn't be spit out as two seperate warnings, but as:
> g_warning ("g_module_supported is deprecated. "
> "modules are always supported by the underlying libltdl.");
> (people also shouldn't use embedded newlines in warnings).
Wasn't aware of that. Sorry.
> but actually i don't think a warning is really apropriate here, we should
> either leave that function in place, always returning TRUE if libtool
> supports modules on _every_ platform, or completely get rid of it and
> cause a compilation warning.
Removing it entirely breaks interface compatibility, and the warning is
inside G_ENABLE_DEBUG, so it is effectively an unconditional TRUE return for
production code. I think this is already the right solution.
> > GModule*
> > g_module_open (const gchar *file_name,
> > GModuleFlags flags)
>
> you do the loading of the module and it's init call not within
> a common lock being held, this is not going to work if two
> threads concurrently try to load the same module, as _init
> could be called twice here.
Good call. Fixed.
> also you don't do any reference counting in GModule itself
> anymore which is a problem wrg track-keeping of the init function.
libltdl keeps track of the reference count for me. I can take a look at the
current count with:
lt_dlinfo *info = lt_dlgetinfo (handle);
gint ref_count = info->ref_count;
> [[snip]]
> important constraints are:
> - before any module coded is used, g_module_check_init() is called
> - g_module_check_init() is called only once
> - after any module code was executed, g_module_unload() is called
> so the module can do proper cleanup
It seems I didn't pay enough attention to the semantics in the HEAD revision.
However, I believe the attached version satisfies these constraints now.
> [[snip]]
> (another implication just sprang to my mind, GModule doesn't currently
> handle static C++ constructors
Nor does libltdl, since lt_dlsym() doesn't really work with the C++ name
mangler. Unfortunately, this means that the dlopen concept doesn't really
port to C++. I can imagine a hairy workaround, but I don't inflict C++ on
myself outside of work *grin* =)O|
> > g_module_init_once (); /* Is this really neccessary? */
>
> no, you can't have a valid module handle without g_module_open(),
> which did initialization already.
True. Thanks!
> > info = lt_dlgetinfo ((lt_dlhandle) module);
> >
> > g_return_val_if_fail (info->ref_count > 1, FALSE);
> >
> > if ((info->ref_count == 1) && !lt_dlisresident ((lt_dlhandle) module))
>
> eh? i never see you doing a decrement.
No need, I'm looking at the ref_count maintained for me by libltdl.
> > return g_strdup (info->filename ? info->filename : "main");
> > }
>
> this did not return a newly allocated string and shouldn't
> in your version either.
Hmm. Where did I get that from? Doh!
> > gchar*
> > g_module_build_path (const gchar *directory,
> > const gchar *module_name)
> > {
>
> how's libtool going to handle "libfoo.la" vs. "libfoo.so" vs. "libfoo.dll"
> vs. "libfoo" vs. "foo" arguments being passed into open()?
lt_dlopenext("libfoo") (as used by g_module_open()) adds the host platform's
module extension, if dlopening of the exact argument string failed. So,
rather than:
g_module_open (g_module_build_path (NULL, "libfoo"));
You would use:
g_module_open ("libfoo");
and libltdl takes care of the rest (although both work, since the literal
string is tried first).
Cheers,
Gary.
--
___ _ ___ __ _ mailto: gvv techie com
/ __|__ _ _ ___ _| | / / | / /_ _ _ _ __ _| |_ __ _ ___ gary gnu org
| (_ / _` | '_|// / |/ /| |/ / _` | || / _` | ' \/ _` | _ \
\___\__,_|_|\_, /|___(_)___/\__,_|\_,_\__, |_||_\__,_|//_/
home page: /___/ /___/ gpg public key:
http://www.oranda.demon.co.uk http://www.oranda.demon.co.uk/key.asc
/* GMODULE - GLIB wrapper code for dynamic module loading
* Copyright (C) 1998 Tim Janik
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the
* Free Software Foundation, Inc., 59 Temple Place - Suite 330,
* Boston, MA 02111-1307, USA.
*/
/*
* Modified by the GLib Team and others 1997-2000. See the AUTHORS
* file for a list of people on the GLib Team. See the ChangeLog
* files for a list of changes. These files are distributed with
* GLib at ftp://ftp.gtk.org/pub/gtk/.
*/
#ifndef __GMODULE_H__
#define __GMODULE_H__
#include <glib.h>
G_BEGIN_DECLS
/* exporting and importing functions, this is special cased
* to feature Windows dll stubs.
*/
#define G_MODULE_IMPORT extern
#if defined (G_OS_WIN32)
# define G_MODULE_EXPORT __declspec(dllexport)
#else /* !G_OS_WIN32 */
# define G_MODULE_EXPORT
#endif /* !G_OS_WIN32 */
/* No longer used. These remain for interface compatibility only. */
typedef enum
{
G_MODULE_BIND_LAZY = 1 << 0,
G_MODULE_BIND_MASK = 0x01
} GModuleFlags;
typedef gpointer GModule;
typedef const gchar* (*GModuleCheckInit) (GModule *module);
typedef void (*GModuleUnload) (GModule *module);
/* open a module `file_name' and return handle, which is NULL on error */
GModule* g_module_open (const gchar *file_name,
GModuleFlags flags);
/* close a previously opened module, returns TRUE on success */
gboolean g_module_close (GModule *module);
/* make a module resident so g_module_close on it will be ignored */
void g_module_make_resident (GModule *module);
/* query the last module error as a string */
gchar* g_module_error (void);
/* retrive a symbol pointer from `module', returns TRUE on success */
gboolean g_module_symbol (GModule *module,
const gchar *symbol_name,
gpointer *symbol);
/* retrive the file name from an existing module */
gchar* g_module_name (GModule *module);
/* Reset the list of colon separated module search directories. */
gchar* g_module_set_search_path (gchar *path);
/* Add a new search directory to the list. */
gchar* g_module_add_search_path (gchar *path);
/* --- deprecated functions --- */
gboolean g_module_supported (void) G_GNUC_CONST;
gchar* g_module_build_path (const gchar *directory,
const gchar *module_name);
G_END_DECLS
#endif /* __GMODULE_H__ */
/* GMODULE - GLIB wrapper code for dynamic module loading
* Copyright (C) 1998 Tim Janik
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 2 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this library; if not, write to the
* Free Software Foundation, Inc., 59 Temple Place - Suite 330,
* Boston, MA 02111-1307, USA.
*/
/*
* Modified by the GLib Team and others 1997-2000. See the AUTHORS
* file for a list of people on the GLib Team. See the ChangeLog
* files for a list of changes. These files are distributed with
* GLib at ftp://ftp.gtk.org/pub/gtk/.
*/
#include "gmodule.h"
#include "gmoduleconf.h"
#include "ltdl.h"
#include <errno.h>
#include <string.h>
#define G_CALLERID_UNDEFINED ((lt_dlcaller_id) -1)
#define G_MODULE_INIT_STRING "g_module_check_init"
#define G_MODULE_FINI_STRING "g_module_unload"
/* --- prototypes --- */
static inline gboolean g_module_init_once (void);
static inline void g_module_set_error (const gchar *error);
static const gchar *g_module_get_error (void);
static void g_module_mutex_lock (void);
static void g_module_mutex_unlock (void);
/* --- variables --- */
G_LOCK_DEFINE_STATIC (GModule);
static GStaticPrivate module_error_private = G_STATIC_PRIVATE_INIT;
/* --- inline functions --- */
static inline void
g_module_set_error (const gchar *error)
{
g_static_private_set (&module_error_private, error, NULL);
errno = 0;
}
/* For orthogonality only: g_module_error already does exactly what
* is required, but is named according to the supported API.
*/
#define g_module_get_error g_module_error
static inline gboolean
g_module_init_once (void)
{
static gboolean initialized = FALSE;
/* Clear error message for this thread. */
g_module_set_error (NULL);
G_LOCK (GModule);
if (!initialized)
{
gint errors = 0; /* Number of errors from libltdl calls. */
initialized = TRUE; /* No need to do this again. */
/* Make sure libltdl is using our memory managment API. */
lt_dlmalloc = (lt_ptr(*)(size_t)) g_malloc;
lt_dlfree = (void (*)(lt_ptr)) g_free;
/* Register the callbacks to make all lt_*() functions MT safe. */
lt_dlmutex_register (g_module_mutex_lock, g_module_mutex_unlock,
g_module_set_error, g_module_get_error);
/* Initialise ltdl library. */
errors = lt_dlinit ();
if (!errors)
{
gchar *path = g_getenv ("GMODULE_PATH");
/* Create a gmodule specific key for subsequent access to
* per-module data stored by gmodule functions.
*/
g_module_caller_id = lt_dlcaller_register ();
/* Initialise libltdl module search directory list. */
if (path != NULL)
errors = lt_dladdsearchdir (path);
}
G_UNLOCK (GModule);
return (errors == 0);
}
G_UNLOCK (GModule);
return TRUE;
}
/* --- static functions --- */
static void
g_module_mutex_lock (void)
{
G_LOCK (GModule);
}
static void
g_module_mutex_unlock (void)
{
G_UNLOCK (GModule);
}
/* --- exported functions --- */
/**
* g_module_supported:
*
* This function is deprecated.
* Check if modules are supported on the current platform. Now that
* the gmodule API is a wrapper for libltdl, this function will always
* return %TRUE.
*
* Return value: %TRUE if modules are supported.
**/
gboolean
g_module_supported (void)
{
#if G_ENABLE_DEBUG
static gboolean first_call = TRUE;
if (first_call)
{
g_warning ("g_module_supported is deprecated. "
"modules are always supported by the underlying libltdl.");
first_call = FALSE;
}
#endif /* G_ENABLE_DEBUG */
return TRUE;
}
/**
* g_module_open:
* @filename: the name of the file containing the module to be opened.
* @flags: not used.
*
* Opens a module. If the module has already been opened, its reference
* count is incremented.
*
* First of all g_module_open() tries to open @file_name as a module. If
* that fails and @file_name has the ".la"-suffix (and is a libtool archive)
* it tries to open the corresponding module. If that fails and @file_name
* doesn't have the proper module suffix for the host platform, this suffix
* will be appended and any corresponding module opened. If that fails and
* @file_name doesn't have the ".la"-suffix, it is appended and
* g_module_open() tries to open the corresponding module. If ultimately
* that fails as well, %NULL is returned.
*
* Return value: a #GModule on success, or %NULL on failure.
**/
GModule*
g_module_open (const gchar *file_name,
GModuleFlags flags)
{
lt_dlhandle handle = (lt_dlhandle) 0;
gchar *error = NULL;
lt_dlinfo *info = NULL;
g_module_init_once ();
G_LOCK (GModule);
/* open the module */
handle = lt_dlopenext (file_name);
if (handle)
info = lt_dlgetinfo (handle);
/* Don't try to run the _init function, except when this is the first
* time this module has been loaded.
*/
if (info && (info->ref_count == 1))
{
GModuleCheckInit check_init;
GModuleUnload unload;
const gchar *check_failed_error = NULL;
/* check initialization */
if (g_module_symbol ((GModule *) handle, G_MODULE_INIT_STRING,
(gpointer) &check_init))
{
check_failed_error = check_init ((GModule *) handle);
}
if (check_failed_error != NULL)
{
error = g_strconcat ("GModule initialization check failed: ",
check_failed_error, NULL);
}
}
G_UNLOCK (GModule);
/* Register error diagnostic generated above, if any. */
if (error)
{
g_module_close ((GModule *) handle);
handle = (lt_dlhandle) 0;
g_module_set_error (error);
g_free (error);
}
return (GModule *) handle;
}
/**
* g_module_close:
* @module: the module to be closed.
*
* Closes an open module.
*
* Return value: %TRUE on success.
**/
gboolean
g_module_close (GModule *module)
{
lt_dlinfo *info;
gboolean success = TRUE;
g_return_val_if_fail (module != NULL, FALSE);
info = lt_dlgetinfo ((lt_dlhandle) module);
g_return_val_if_fail (info->ref_count > 1, FALSE);
if ((info->ref_count == 1) && !lt_dlisresident ((lt_dlhandle) module))
{
GModuleUnload unload;
lt_ptr stale = NULL;
unload = g_module_symbol (module, G_MODULE_FINI_STRING,
(gpointer) &unload);
if (unload)
unload (module);
if (lt_dlclose ((lt_dlhandle) module) != 0)
success = FALSE;
g_free (module);
}
return success;
}
/**
* g_module_make_resident:
* @module: the module to make permenantly resident.
*
* Ensures that a module can never be unloaded. Any future g_module_close()
* calls on the module will have no effect. If you open the entire process
* as a reflexive module, then it is always made resident by default -- there
* is no need to call this function manually.
**/
void
g_module_make_resident (GModule *module)
{
g_return_if_fail (module != NULL);
lt_dlmakeresident ((lt_dlhandle) module);
}
/**
* g_module_error:
*
* Get a string describing the last error that occured in the gmodule
* subsystem.
*
* Return value: a string describing the last module error.
**/
gchar*
g_module_error (void)
{
return g_static_private_get (&module_error_private);
}
/**
* g_module_symbol:
* @module: the module.
* @symbol_name: the name of the symbol to find.
* @symbol: returns the pointer to the symbol value.
*
* Gets a symbol pointer from a module.
*
* Return value: %TRUE on success.
**/
gboolean
g_module_symbol (GModule *module,
const gchar *symbol_name,
gpointer *symbol)
{
if (symbol)
*symbol = NULL;
g_return_val_if_fail (module != NULL, FALSE);
g_return_val_if_fail (symbol_name != NULL, FALSE);
g_return_val_if_fail (symbol != NULL, FALSE);
*symbol = lt_dlsym ((lt_dlhandle) module, symbol_name);
if (*symbol == NULL)
{
gchar *module_error = lt_dlerror ();
gchar *error;
error = g_strconcat ("`", symbol_name, "': ", module_error, NULL);
g_module_set_error (error);
g_free (error);
return FALSE;
}
return TRUE;
}
/**
* g_module_name:
* @module: the module.
*
* Gets the file name from a #GModule.
*
* Return value: the file name of the module, or "main" if the module
* is the main program itself.
**/
const gchar*
g_module_name (GModule *module)
{
lt_dlinfo *info;
g_return_val_if_fail (module != NULL, NULL);
info = lt_dlgetinfo ((lt_dlhandle) module);
return (const gchar *) (info->filename ? info->filename : "main");
}
/**
* g_module_set_search_path:
* @path: a colon delimited list of directories.
*
* Set the paths of the directories to be searced when trying to
* locate a module opened using a relative path.
*
* Return value: %TRUE if successful.
**/
gboolean
g_module_set_search_path (gchar *path)
{
return (lt_dlsetsearchpath (path) == 0);
}
/**
* g_module_add_search_path:
* @path: a colon delimited list of directories.
*
* Append to the list of user paths, the additional @path of directories
* to be searched when trying to locate a module opened using a
* relative path.
*
* Return value: %TRUE if successful.
**/
gboolean
g_module_add_search_path (gchar *path)
{
return (lt_dladdsearchdir (path) == 0);
}
/**
* g_module_build_path:
* @directory: the directory where the module is.
* @module_name: the name of the module.
*
* This function is deprecated.
**/
gchar*
g_module_build_path (const gchar *directory,
const gchar *module_name)
{
#if G_ENABLE_DEBUG
static gboolean first_call = TRUE;
if (first_call)
{
g_warning ("g_module_build_path is deprecated. "
"Use g_module_set_search_path or g_module_add_search_path.");
first_call = FALSE;
}
#endif /* G_ENABLE_DEBUG */
#if (G_MODULE_IMPL == G_MODULE_IMPL_DL)
if (directory && *directory) {
if (strncmp (module_name, "lib", 3) == 0)
return g_strconcat (directory, "/", module_name, NULL);
else
return g_strconcat (directory, "/lib", module_name, ".so", NULL);
} else if (strncmp (module_name, "lib", 3) == 0)
return g_strdup (module_name);
else
return g_strconcat ("lib", module_name, ".so", NULL);
#elif (G_MODULE_IMPL == G_MODULE_IMPL_DLD)
if (directory && *directory)
if (strncmp (module_name, "lib", 3) == 0)
return g_strconcat (directory, "/", module_name, NULL);
else
return g_strconcat (directory, "/lib", module_name, ".sl", NULL);
else if (strncmp (module_name, "lib", 3) == 0)
return g_strdup (module_name);
else
return g_strconcat ("lib", module_name, ".sl", NULL);
#elif (G_MODULE_IMPL == G_MODULE_IMPL_BEOS)
g_warning ("g_module_build_path() untested for BeOS!");
if (directory && *directory)
{
if (strncmp (module_name, "lib", 3) == 0)
return g_strconcat (directory, "/", module_name, NULL);
else
return g_strconcat (directory, "/lib", module_name, ".so", NULL);
}
else if (strncmp (module_name, "lib", 3) == 0)
return g_strdup (module_name);
else
return g_strconcat ("lib", module_name, ".so", NULL);
#elif (G_MODULE_IMPL == G_MODULE_IMPL_WIN32)
{
gint k = strlen (module_name);
if (directory && *directory)
if (k > 4 && g_strcasecmp (module_name + k - 4, ".dll") == 0)
return g_strconcat (directory, "\\", module_name, NULL);
else
return g_strconcat (directory, "\\", module_name, ".dll", NULL);
else if (k > 4 && g_strcasecmp (module_name + k - 4, ".dll") == 0)
return g_strdup (module_name);
else
return g_strconcat (module_name, ".dll", NULL);
}
#elif (G_MODULE_IMPL == G_MODULE_IMPL_OS2)
{
gchar *suffix = strrchr(module_name, '.');
if (directory && *directory)
if (suffix && (stricmp (suffix, ".dll") == 0))
return g_strconcat (directory, "/", module_name, NULL);
else
return g_strconcat (directory, "/", module_name, ".dll", NULL);
else if (suffix && (stricmp (suffix, ".dll") == 0))
return g_strdup (module_name);
else
return g_strconcat (module_name, ".dll", NULL);
}
#endif
return NULL;
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
