Re: e-d-s pokeage ...

On Tue, 2006-02-14 at 11:22 +0000, michael meeks wrote:
> Hi there,
> 
> 	So - I was pleased to see that the libebook .so remains unchanged from
> evo 2.4 to 2.6 - and I just did a little review to try to ensure that
> this indeed reflects an unchanged ABI ;-)
> 
> 	It seems that is the case - which is great thanks - I need only to
> update a comment in OO.o ;-) however, as with all reviews I happened on
> some other things:
> 
> libebook:
> 	+ e-name-western.h: urgh not good practice
> 		+ do we really want to export this structure ?
> 		  surely an opaque type & accessors is ~always 
> 		  better ?
> 
> 	+ e-name-western-tables.h:
> 		+ looks broken to me:
> 		* if you include this header it will instantiate
> 		  this large set of strings in each module / 
> 		  shlib.
> 		* all such lists of strings should prolly be
> 		  defined 'const' so we put them in a shared
> 		  linker section, to save memory.
> 		* I imagine the header should have
> 		  'extern const char *' and just the symbol names
> 		+ why is this installed anyway ? are there 
> 		  better ways to expose this ?

I will let the addressbook hackers comment on it first 


> libedataserver/e-data-server-module.c
> 	+ you add:
> 
> -       module->library = g_module_open (module->path, 0);
> +       module->library = g_module_open (module->path, G_MODULE_BIND_LAZY);
> 
> 	in moving to 2.6 - which is some optimization; of course - it would be
> rather better to use:
> 
> 	(G_MODULE_BIND_LAZY | G_MODULE_BIND_LOCAL)
> 
> 	instead. What that does is ensure that the plugin is not included in
> the search path for symbols for other plugins - ie. subsequent plugins
> as they link don't have to search this plugin for symbols. Assuming your
> plugins don't depend on each other to provide symbols [ pretty broken
> IMHO ] this not only accelerates linking, but prolly helps avoid various
> potential tangled linking problems.


No, the plugins do not depend on each other for symbols - I will take
this in.

> 	HTH & thanks for not breaking the ABI again :-)


:-)

Thanks,
Harish