Re: module cleanup [2/n]

Hi Eric,

Some nits.  I'd have reviewed before you committed, except that I've  
been
offline for almost a week while I relocate to England.

On Sep 7, 2007, at 9:58 PM, Eric Blake wrote:
> 2007-09-06  Eric Blake  <[email protected]>
>
> 	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.

> 	(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...

> 	(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.

> 	(module_close): Delete, and inline into module_remove.

I rather liked the symmetry before this change :-(

> Index: m4/input.c
> ===================================================================
> RCS file: /sources/m4/m4/m4/input.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 input.c
> --- m4/input.c	7 Aug 2007 03:15:26 -0000	1.68
> +++ m4/input.c	7 Sep 2007 20:48:06 -0000
> @@ -163,7 +163,7 @@ struct m4_input_block
>        struct
>  	{
>  	  const m4_builtin *builtin;  /* pointer to builtin's function. */
> -	  lt_dlhandle handle;	  /* originating module. */
> +	  m4_module *handle;	  /* originating module. */

	    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.

Cheers,
	Gary
-- 
   ())_.              Email me: [email protected]
   ( '/           Read my blog: http://blog.azazil.net
   / )=         ...and my book: http://sources.redhat.com/autobook
`(_~)_      Join my AGLOCO Network: http://www.agloco.com/r/BBBS7912

_______________________________________________
M4-patches mailing list
[email protected]
http://lists.gnu.org/mailman/listinfo/m4-patches