-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 According to Gary V. Vaughan on 9/11/2007 4:06 PM: >> >> Wrap lt_dlhandle in struct m4_module. > > Wrapping like this introduces an additional dereference every time a module > is accessed. For this, and aesthetic reasons, I'd rather see m4_module as > a simple typedef alias.
libltdl can only attach a single pointer to the lt_dlhandle. So it has to be a pointer to a struct for us to store more information than that. I'm still not convinced whether handing m4 clients the raw lt_dlhandle (under a type-safe opaque alias) is any more efficient than handing them the pointer to the struct associated with the handle, and having that struct point back to the handle (which is what I implemented). One benefit of my approach is that we can cache anything in the struct, reducing the need to continually rely on libltdl accessors to relearn something that hasn't changed since the last time we checked (for example, the implementation of m4_get_module_name could be sped up by caching lt_dlgetinfo(handle)->name in the m4_module, at which point a fast accessor macro is easier to write). > >> (m4_module): New declaration. Simple for now, but intended for >> growth. > > I would say that you have found a deficiency in the libltdl apis if you > need > to duplicate information that is already tracked by libltdl itself, or that > you are unable to store with lt_dlcaller_set_data... I found that storing the m4_module* using lt_dlcaller_set_data worked all right, once I fixed the lt_dlcaller_get_data bug. > >> (m4_module_makeresident, m4_module_refcount): New functions. > > These should be macros (at least with NDEBUG) that defer to the > identical ltdl > calls, to avoid the extra stack frame. I'll keep that on my list of future cleanups. > >> (module_close): Delete, and inline into module_remove. > > I rather liked the symmetry before this change :-( This whole area is still a bit awkward - particularly since both functions were having to strdup the module name in case of error. You're right that there are still some symmetry issues to work out, as well as correctly dividing the work between the three layers: m4_module_import/m4_module_? - opens a module in order to import an entry point from it (for example, libgnu's esyscmd needs to affect libm4's sysval builtin) - this currently affects the m4 symbol table, but I'm not sure that it should, and I don't know if a symmetric partner is needed m4_module_load/m4_module_unload - opens modules and affects m4 symbol table, designed for use by libload and other m4 modules m4__module_open/m4__module_? - opens a module, but leaves m4 symbol table alone, designed for use by internals, such as src/freeze.c static module_?/static module_remove - handles the libltdl handle open and close, designed for use only by module.c I'll think about it some more, and try to clean it up better. > m4_module *module; > > ...would make it easier to distinguish between handles (ld_dlhandle) and > modules > (m4_module *) on the boundaries between the ltdl and m4module code. Thanks for doing that cleanup patch. - -- Don't work too hard, make some time for fun as well! Eric Blake [EMAIL PROTECTED] -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.5 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFG6S+j84KuGfSFAYARAgM5AJ47lypL8ECv86uOmSf1c9MTE3FwEACg2QUd Zmzwtq2dmW7dkD1DmOU8MHE= =sd/H -----END PGP SIGNATURE----- _______________________________________________ M4-patches mailing list [email protected] http://lists.gnu.org/mailman/listinfo/m4-patches
