Re: URLClassLoader update

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:

Mark> The last couple of weeks I have rewritten the URLClassLoader
Mark> implementation based on the old GNU Classpath version and the
Mark> changes that the Orp developers made. This version has different
Mark> Loaders for the different types of URLs that can be given to the
Mark> URLClassLoader (URLClassLoader now works for remote locations! -
Mark> now if only we had a complete verifier and security permissions
Mark> framework...).

Wow, this is a complicated patch.  Overall it looks good to me.

I have a few questions.

Mark> [ ClassLoader ]
Mark>    /**
Mark> +   * Normalizes a resource path name. This removes all ./ from the start and
Mark> +   * all further '//', "/../" and "/./" from the given name.
Mark> +   */

I don't understand why this is a valid operation.  For instance, if
the resource comes from the file system, then `..' removal is
incorrect in the presence of symlinks.  Maybe it doesn't matter
though -- a plausible argument.

Mark> [ URLClassLoader ]
Mark> +	    // Check if it's a jar url
Mark> +	    if (!(file.endsWith("/") || file.endsWith(File.separator)))
Mark> +	      loader = new JarURLLoader(this, newUrl);

I missed that `!' the first few times through this...
speaking of which there are various formatting buglets in this code.

Mark> +  protected Class findClass(final String className)
Mark> +    throws ClassNotFoundException
Mark> +  {
Mark> +    // Just try to find the resource by the (almost) same name
Mark> +    String resourceName = className.replace('.', '/') + ".class";
Mark> +    Resource resource = findURLResource(resourceName);

I have a patch here with this hunk:

@@ -266,7 +274,7 @@
 
     try 
       {
-	URL url = getResource (name.replace ('.', '/') + ".class");
+	URL url = findResource (name.replace ('.', '/') + ".class");
 
 	if (url == null)
 	  throw new ClassNotFoundException (name);

Should findClass be calling findResource instead?  The rationale is
that a subclass might override just findResource.  Eclipse does this.

Tom