Re: [cp-patches] FYI Only truncate a file on append and write-only (from FileOutputStream)

[Steve Blackburn]

Is there any particular reason for having native implementations of  
open() in both gnu_java_nio_VMChannel.c and  
gnu_java_nio_FileChannelImpl.c?  Aside from the bug in VMChannel,  
this duplication of open seemed to be the cause of the problem I saw.

Refactoring of FileChannelImpl.java lead to an (unintended?) switch  
between open() implementations and exposed the bug. I couldn't work  
out why there were two implementations in the first place.

--Steve

On 30/05/2007, at 9:12 PM, Andrew Haley wrote:

You know, the real cause of this bug was the obfuscated use of
conditional expressions.  This code does, I think, the same things,
but it's hard to be sure.

Andrew.


Index: gnu_java_nio_VMChannel.c
===================================================================
RCS file: /cvsroot/classpath/classpath/native/jni/java-nio/ 
gnu_java_nio_VMChannel.c,v
retrieving revision 1.19
diff -c -2 -p -w -r1.19 gnu_java_nio_VMChannel.c
*** gnu_java_nio_VMChannel.c    30 May 2007 09:56:57 -0000      1.19
--- gnu_java_nio_VMChannel.c    30 May 2007 11:04:57 -0000
*************** Java_gnu_java_nio_VMChannel_open (JNIEnv
*** 1667,1683 ****
    const char *npath;

!   if ((mode & CPNIO_READ) && (mode & CPNIO_WRITE))
!     nmode = O_RDWR;
!   else if (mode & CPNIO_WRITE)
!     nmode = O_WRONLY;
!   else
      nmode = O_RDONLY;

!   nmode = (nmode
!            | ((nmode == O_RDWR || nmode == O_WRONLY) ? O_CREAT : 0)
!            | ((mode & CPNIO_APPEND) ? O_APPEND :
!               ((nmode == O_WRONLY) ? O_TRUNC : 0))
!            | ((mode & CPNIO_EXCL) ? O_EXCL : 0)
!            | ((mode & CPNIO_SYNC) ? O_SYNC : 0));

    npath = JCL_jstring_to_cstring (env, path);
--- 1667,1700 ----
    const char *npath;

!   switch (mode & (CPNIO_READ | CPNIO_WRITE | CPNIO_APPEND))
!     {
!     case 0:
!     case CPNIO_APPEND: /* Is this legal?  */
!       break;
!     case CPNIO_WRITE:
!       nmode = O_WRONLY | O_CREAT | O_TRUNC;
!       break;
!     case CPNIO_WRITE | CPNIO_APPEND:
!       nmode = O_WRONLY | O_CREAT | O_APPEND;
!       break;
!     case CPNIO_READ:
        nmode = O_RDONLY;
+       break;
+     case CPNIO_READ | CPNIO_APPEND:
+       nmode = O_RDONLY | O_APPEND; /* Is this legal?  */
+       break;
+     case CPNIO_READ | CPNIO_WRITE:
+       nmode = O_RDWR | O_CREAT | O_TRUNC;
+       break;
+     case CPNIO_READ | CPNIO_WRITE | CPNIO_APPEND:
+       nmode = O_RDWR | O_CREAT | O_APPEND;
+       break;
+     }
+
+   if (mode & CPNIO_EXCL)
+     nmode |= O_EXCL;

!   if (mode & CPNIO_SYNC)
!     nmode |= O_SYNC;

    npath = JCL_jstring_to_cstring (env, path);