Re: get/set_prop patch

[Michael Meeks <michael ximian com>]

Hi Alex,

On Tue, 26 Jun 2001, Alex Larsson wrote:
> After some profiling of Nauilus and discussion with Michael i have
> come up with a patch that basically removes Bonobu UI handler from the
> Nautilus open new window profiles.

	The patch looks great overall, just a few comments:

> +	full_path = g_malloc (strlen (path) + 1 + strlen (prop) + 1);

	I'm a fan of alloca, this is no problem though.

> +		if ((property = strrchr (path, '/')) &&
> +		    (property = strrchr (property, '#'))) {

	I wonder why you do the strrchr (path, '/') - it doesn't seem
instantly useful to me - and again in the 'get'

> +		property = property + 1; /* Skip the '#' */
> +
> +		xml = bonobo_ui_engine_xml_get_prop (engine, real_path, property);
> +		g_free (real_path);

	you can't return NULL from a CORBA method [ for a string ] without
getting a seg fault, so ... perhaps you want to split out the exception
set below for both branches:

> +		xml = bonobo_ui_engine_xml_get (engine, path, nodeOnly);
> +		if (!xml) {
> +			CORBA_exception_set (ev, CORBA_USER_EXCEPTION,
> +					     ex_Bonobo_UIContainer_InvalidPath, NULL);
> +			return NULL;
> +		}

>  CORBA_char      *bonobo_ui_engine_xml_get         (BonoboUIEngine   *engine,
>  						   const char       *path,
>  						   gboolean          node_only);
> +CORBA_char *     bonobo_ui_engine_xml_get_prop    (BonoboUIEngine   *engine,

	if you could knock the '*' across that'd be sweeter.

	Apart from that: lovely.

> It is a bit hacky, but it is binary compatible. We should probably
> solve this in another way in Bonobo 2.

	Yes, any ideas / patches would be much appreciated.

> This patch might also help evolution performance, and any app that
> does a lot of bonobo_ui_component_set_prop.

	Which is wonderful - thanks again for all the work you've put in,
if you could address the above and commit that'd be great - I'll do a new
release shortly - but I'd like to get Darin's patch in too - Darin ?

	Regards,

		Michael.

-- 
 mmeeks gnu org  <><, Pseudo Engineer, itinerant idiot