This is the mail archive of the java-patches@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Repost: lock/unlock filechannel Win32 implementation (patch file)


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/




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]