[Mono-dev] Support for runtime configuration of assembly loading path - related to bug #81446

[Paolo Molaro]

On 08/23/07 Marek Habersack wrote:
> <?xml version="1.0"?>
> <configuration>
>     	<runtime>
>         	<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
> 			<probing privatePath="test:libraries" />
> 		</assemblyBinding> 
> 	</runtime>
> </configuration>

> Index: mono/metadata/appdomain.c
> ===================================================================
> --- mono/metadata/appdomain.c	(revision 84664)
> +++ mono/metadata/appdomain.c	(working copy)
[...]
>  static gunichar2 process_guid [36];
> @@ -484,6 +491,114 @@
>  	return root->domain;
>  }
>  
> +static
> +char* get_attribute_value (const gchar **attribute_names, 

static char*
get_attribute_value (...)

> +			   const gchar **attribute_values, 
> +			   const char *att_name)
> +{
> +	int n;
> +	for (n=0; attribute_names[n] != NULL; n++) {

You need spaces around = and before an opening paren.

> +		if (strcmp (attribute_names[n], att_name) == 0)
> +			return g_strdup (attribute_values[n]);
> +	}
> +	return NULL;
> +}
> +
> +static void start_element (GMarkupParseContext *context, 

static void on its own line.

> +                           const gchar         *element_name,
> +			   const gchar        **attribute_names,

The args are aligned with random amounts of tabs and spaces.

> +			   const gchar        **attribute_values,
> +			   gpointer             user_data,
> +			   GError             **error)
> +{
> +	RuntimeConfig *runtime_config = (RuntimeConfig*) user_data;

Remove the useless cast.

> +	if (strcmp (element_name, "runtime") == 0) {
> +		runtime_config->runtime_count++;
> +		return;
> +	}
> +
> +	if (strcmp (element_name, "assemblyBinding") == 0) {
> +		runtime_config->assemblybinding_count++;
> +		return;
> +	}
> +	
> +	if (runtime_config->runtime_count != 1 || runtime_config->assemblybinding_count != 1)
> +		return;
> +
> +	if (strcmp (element_name, "probing") != 0)
> +		return;
> +
> +	runtime_config->domain->private_bin_path = get_attribute_value (attribute_names, attribute_values, "privatePath");

If private_bin_path had a value it is leaked.

> +static void
> +mono_set_private_bin_path_from_config (MonoDomain *domain)
> +{
> +	gchar *config_file, *text, *runtime_start;
> +	gsize len;
> +	struct stat sbuf;
> +	GMarkupParseContext *context;
> +	RuntimeConfig *runtime_config;
> +	
> +	if (!domain || !domain->setup || !domain->setup->configuration_file)
> +		return;
> +
> +	config_file = mono_string_to_utf8 (domain->setup->configuration_file);
> +	if (stat (config_file, &sbuf) != 0) {
> +		g_free (config_file);
> +		return;
> +	}
> +
> +	if (!g_file_get_contents (config_file, &text, &len, NULL)) {
> +		g_free (config_file);
> +		return;
> +	}
> +	g_free (config_file);
> +
> +	runtime_start = strstr (text, "<runtime>");
> +	if (!runtime_start) {
> +		g_free (text);
> +		return;
> +	}

Please remove this questionable (because it is buggy) hack.

> +	runtime_config = g_new0 (RuntimeConfig, 1);

No need to alloc/free this struct, just put it in the stack as a local var.

> +	runtime_config->domain = domain;
> +	
> +	context = g_markup_parse_context_new (&mono_parser, 0, runtime_config, NULL);
> +	if (g_markup_parse_context_parse (context, runtime_start, len - (runtime_start - text), NULL))
> +		g_markup_parse_context_end_parse (context, NULL);

XML files need to parsed from the start, not from a ranom spot.
Please also make sure no new warning are added to the compilation and
that the test case actually runs.

Thanks!

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better