[Mono-dev] [Patch] AssemblyName ctor (with corrections)

[Carlos Alberto Cortez]

Hey Paolo,

Thanks for the comments.

Carlos.

El mié, 14-09-2005 a las 20:45 +0200, Paolo Molaro escribió:
> On 09/01/05 Carlos Alberto Cortez wrote:
> > Index: AssemblyName.cs
> > ===================================================================
> > --- AssemblyName.cs	(revisión: 48811)
> > +++ AssemblyName.cs	(copia de trabajo)
> > @@ -37,6 +37,7 @@
> >  using System.Text;
> >  using System.Runtime.InteropServices;
> >  using System.Runtime.CompilerServices;
> > +using System.IO;
> >  
> >  using Mono.Security;
> >  
> > @@ -80,12 +81,23 @@
> >  			versioncompat = AssemblyVersionCompatibility.SameMachine;
> >  		}
> >  
> > -#if NET_2_0
> > +#if NET_2_0 || BOOTSTRAP_NET_2_0
> > +		[MethodImpl (MethodImplOptions.InternalCall)]
> > +		static extern bool ParseName (AssemblyName aname, string assemblyName);
> > +		
> >  		public AssemblyName (string assemblyName)
> >  		{
> > -			name = assemblyName;
> > +			if (assemblyName == null)
> > +				throw new ArgumentNullException ("assemblyName");
> > +			if (assemblyName.Length < 1)
> > +				throw new ArgumentException ("assemblyName cannot have zero length.");
> > +			
> > +			if (!ParseName (this, assemblyName))
> > +				throw new FileLoadException ("The assembly name is invalid.");
> 
> Is a FileLoadException really ok? Do you have tests for this?
> 
> > Index: assembly.c
> > ===================================================================
> > --- assembly.c	(revisión: 48811)
> > +++ assembly.c	(copia de trabajo)
> > @@ -1171,10 +1171,92 @@
> >  	g_free ((void *) aname->hash_value);
> >  }
> >  
> > +static gint 
> > +to_int32_le (gchar bytes [], gint offset)
> > +{
> > +	return (bytes [offset+3] << 24) | (bytes [offset+2] << 16) | (bytes [offset+1] << 8) | bytes [offset];
> > +}
> > +
> 
> The function name looks wrong. You should likely use read32() anyway.
> 
> >  static gboolean
> > -build_assembly_name (const char *name, const char *version, const char *culture, const char *token, MonoAssemblyName *aname)
> > +parse_public_key (const gchar *key, const gchar** pubkey)
> >  {
> [...]
> > +	if (key && strncmp (key, "null", 4) != 0) {
> > +		if (!parse_public_key (key, (const gchar**) &pkey)) {
> 
> Why declare parse_public_key() to take a const gchar** when it's not in
> the only place it's called?
> 
> > +
> > +		if (!g_ascii_strncasecmp (value, "PublicKey=", 10)) {
> > +			key = g_strstrip (value + 10);
> 
> Where is key freed?
> 
> > +			tmp++;
> > +			continue;
> > +		}
> >  		
> >  		g_strfreev (parts);
> >  		return FALSE;
> >  	}
> >  
> > -	res = build_assembly_name (dllname, version, culture, token, aname);
> > +	res = build_assembly_name (dllname, version, culture, token, key, aname, save_public_key);
> >  	g_strfreev (parts);
> >  	return res;
> >  }
> [...]
> > Index: icall.c
> > ===================================================================
> > --- icall.c	(revisión: 48811)
> > +++ icall.c	(copia de trabajo)
> > @@ -4152,9 +4152,11 @@
> >  		mono_method_desc_free (desc);
> >  	}
> >  
> > -	args [0] = mono_string_new (domain, name->culture);
> > -	aname->cultureInfo = 
> > -		mono_runtime_invoke (create_culture, NULL, args, NULL);
> > +	if (name->culture) {
> > +		args [0] = mono_string_new (domain, name->culture);
> > +		aname->cultureInfo = 
> > +			mono_runtime_invoke (create_culture, NULL, args, NULL);
> 
> Put on the same line.
> 
> Thanks.
> Once you'll have fixed the remaining issues, please commit.
> 
> lupus
>