Re: FYI: Patch: file stuff

[Mark Wielaard]

Hi,

On Sat, 2004-01-10 at 23:49, Michael Koch wrote:
> I wanna start a new trend on this mailing list now. I wanna ask for approving 
> the attached patch. This patch fixes two bugs with files.

Good practice, but Don't start your subject with FYI (For Your
Information) in that case. That makes it look like you don't expect a
patch review.

> 1) java.io.File.list() uses an internal method called listInternal(). This 
> methods returns null in the error case. list() now converts this to a 
> String[0] array which is similar to an empty directory. This behaviour is 
> incorrect. We should return the null in the error case.

Correct. When the given file isn't a directory the spec says we must
return null (is there a Mauve test for this?). Better also update the
documentation of listInternal() to mention this fact.

> 2) Java_java_io_File_listInternal doest release local references to a newly 
> created string. This patch fixes this.

Good catch. Some JNI implementations limit the number of local
references so this could easily bite someone.

> Please review and comment.
> 
> mjw: Okay to commit ?

Yes, please do. If you can also look after adding the comment to
listInternal() and checking whether we have a Mauve test for this case,
that would be nice.

Note that others with commit-access can also approve patches if they
want. In general it is a good idea when the original author of the code
that gets changed by a patch takes a look at it.

Cheers,

Mark

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