Hi Olivier,

On Mon, 2006-03-20 at 20:27 +0100, Olivier Jolly wrote:
>   here is attached another version of the patch rewrote with 
> StringTokenizer and an iterative algorithm. It still passes all the 
> mauve test about resources and looks more efficient.

Yes. Thanks please do check this in with the original ChangeLog entry.
Some nitpicks below.

> Index: URLClassLoader.java
> ===================================================================
> RCS file: /sources/classpath/classpath/java/net/URLClassLoader.java,v
> retrieving revision 1.45
> diff -u -r1.45 URLClassLoader.java
> --- URLClassLoader.java 5 Mar 2006 17:20:51 -0000       1.45
> +++ URLClassLoader.java 20 Mar 2006 19:12:45 -0000
> @@ -538,6 +538,11 @@
>      {
>        try 
>         {
> +
> +      // Make sure that all components in name are valid by walking through 
> them
> +      if (!checkFileValidity(name))
> +        return null;

Indenting looks wrong here.
      
>           File file = new File(dir, name).getCanonicalFile();
>           if (file.exists())
>             return new FileResource(this, file);

Small optimization would be to push this into checkFileValidity() and
make this return the (canonicalized) File or null when not valid.

> +      // Treat the last token differently, if it exists, because it does not 
> need
> +      // to be a directory
> +      if (tokenCount > 0)
> +        {
> +          String currentToken = stringTokenizer.nextToken();
> +          
> +          if ("..".equals(currentToken) && currentFile.equals(dir))
> +            return false;          
> +        }

Then here you also check currentFile.exists() and were you return false,
you return null and at the end of the method you would return
currentFile.getCanonicalFile() so you don't have to do that in the
calling getResource(). But that is probably micro-optimizing (to get rid
of one extra File object creation), so feel free to ignore this
suggestion.

Cheers,

Mark

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to