This is the mail archive of the
java-patches@gcc.gnu.org
mailing list for the Java project.
Re: Repost: lock/unlock filechannel Win32 implementation (patch file)
- From: Mohan Embar <gnustuff at thisiscool dot com>
- To: Michele Sandri <gpointorama at gmail dot com>
- Cc: java-patches at gcc dot gnu dot org
- Date: Sun, 28 Jan 2007 00:44:53 -0600
- Subject: Re: Repost: lock/unlock filechannel Win32 implementation (patch file)
- Reply-to: gnustuff at thisiscool dot com
Hi Michele,
(Be sure to CC java-patches too so we have a record
of everything.)
>I hope the layout now is ok.
>
>The test case is really simple, but i've tested with
>http://freenetproject.org/ that uses Sleepycat (Berkeley db for java).
Thanks for resubmitting this.
Could you make four small changes to this patch? The
reason I'm being so pedantic about this is because I
also like it when people are like that with me.
- For the ChangeLog, I believe the correct way of saying
that you've implemented a new method looks like this (taken
from an actual ChangeLog entry):
* gnu/awt/xlib/XToolkit.java
(getLocalGraphicsEnvironment): Implemented.
- Could you keep the return value and method name on separate
lines, as per the other methods in natFileChannelWin32.cc?
This is due to some arcane GNU convention which we don't necessarily
respect in the rest of the Win32 code, but I think it's nice
to keep conventions consistent within a single source file.
- For the same reasons, can you decrease the indent size from four
to two spaces for consistency with the rest of the file?
- Finally, could you eliminate the bounds checks (and therefore
the import of IllegalArgumentException) from the code? As far
as I can see, these aren't necessary and aren't present in the
POSIX version.
For some reason, I wasn't able to apply the patch (it complained
about a malformed line) and had to apply it manually. Not quite
sure why this was.
I ran the test against 4.2 and it prints the expected output. I'm
not terribly impressed with the test, but I think the patch looks
fine and with the above changes, can commit this to 4.2 and the
trunk. Thanks again for doing this!
-- Mohan
http://www.thisiscool.com/
http://www.animalsong.org/