harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Tim Ellison <t.p.elli...@gmail.com>
Subject [classlib][luni] calling mmap (was: Re: svn commit: r815994 ...)
Date Tue, 29 Sep 2009 14:03:20 GMT
Sorry for talking so long to raise this...

On 16/Sep/2009 23:44, odeakin@apache.org wrote:
> Author: odeakin
> Date: Wed Sep 16 22:44:49 2009
> New Revision: 815994
> 
> URL: http://svn.apache.org/viewvc?rev=815994&view=rev
> Log:
> Apply patch for HARMONY-6315 ([classlib][nio] FileChannel.map throws IOException when
called with size 0)
> 
> Modified:
>     harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSMemory.java
>     harmony/enhanced/classlib/trunk/modules/luni/src/main/native/luni/unix/OSMemoryLinux32.c
>     harmony/enhanced/classlib/trunk/modules/nio/src/test/java/common/org/apache/harmony/nio/tests/java/nio/MappedByteBufferTest.java
> 
> Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSMemory.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSMemory.java?rev=815994&r1=815993&r2=815994&view=diff
> ==============================================================================
> --- harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSMemory.java
(original)
> +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/java/org/apache/harmony/luni/platform/OSMemory.java
Wed Sep 16 22:44:49 2009
> @@ -551,6 +551,11 @@
>  
>  	public long mmap(long fileDescriptor, long alignment, long size,
>  			int mapMode) throws IOException {
> +		// if size is 0, call to mmap will fail, so return dummy address
> +		if (size == 0)
> +		{
> +			return 0;
> +		}
>  		long address = mmapImpl(fileDescriptor, alignment, size, mapMode);
>  		if (address == -1) {
>  			throw new IOException();

Hmm, I'm not convinced this is the right solution.

Making size == 0 a special case that always succeeds (even if it is
returning a bogus address) bypasses all the other checks that the mmap
function call would have made.

For example, what if the file descriptor was invalid? or points to an
unmappable file?  Then I would expect an IOException.

There is nothing in the man pages to say that size = 0 is invalid.

Plus, there is now no path that will return -1 as the address, so you
can remove the check (all the exceptions are raised in the native code).

Regards,
Tim

> Modified: harmony/enhanced/classlib/trunk/modules/luni/src/main/native/luni/unix/OSMemoryLinux32.c
> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/luni/src/main/native/luni/unix/OSMemoryLinux32.c?rev=815994&r1=815993&r2=815994&view=diff
> ==============================================================================
> --- harmony/enhanced/classlib/trunk/modules/luni/src/main/native/luni/unix/OSMemoryLinux32.c
(original)
> +++ harmony/enhanced/classlib/trunk/modules/luni/src/main/native/luni/unix/OSMemoryLinux32.c
Wed Sep 16 22:44:49 2009
> @@ -26,9 +26,11 @@
>  #include <sys/mman.h>
>  #include <errno.h>
>  #include <unistd.h>
> +#include <string.h>
>  #include "vmi.h"
>  #include "OSMemory.h"
>  #include "IMemorySystem.h"
> +#include "exceptions.h"
>  
>  #ifdef ZOS
>  #define FD_BIAS 1000
> @@ -157,6 +159,7 @@
>    //PORT_ACCESS_FROM_ENV (env);
>    void *mapAddress = NULL;
>    int prot, flags;
> +  char errorString[100];
>  
>    // Convert from Java mapping mode to port library mapping mode.
>    switch (mmode)
> @@ -174,12 +177,41 @@
>  	flags = MAP_PRIVATE;
>          break;
>        default:
> +    sprintf(errorString, "Map mode %d not recognised", mmode);
> +    throwJavaIoIOException(env, errorString);
>          return -1;
>      }
>  
>    mapAddress = mmap(0, (size_t)(size&0x7fffffff), prot, flags, fd-FD_BIAS, (off_t)(alignment&0x7fffffff));
>    if (mapAddress == MAP_FAILED)
>      {
> +      switch (errno)
> +        {
> +        case EACCES:
> +          strcpy(errorString, "Call to mmap failed - a file descriptor refers to a non-regular
file.");
> +          break;
> +        case EAGAIN:
> +          strcpy(errorString, "Call to mmap failed with error EAGAIN - the file has
been locked, or too much memory has been locked.");
> +          break;
> +        case EBADF:
> +          strcpy(errorString, "Call to mmap failed with error EBADF - invalid file descriptor");
> +          break;
> +        case EINVAL:
> +          strcpy(errorString, "Call to mmap failed with error EINVAL - invalid start,
length or offset.");
> +          break;
> +        case ENFILE:
> +          strcpy(errorString, "Call to mmap failed with error ENFILE - number of open
files has reached the system limit.");
> +          break;
> +        case ENODEV:
> +          strcpy(errorString, "Call to mmap failed with error ENODEV - filesystem does
not support memory mapping.");
> +          break;
> +        case ENOMEM:
> +          strcpy(errorString, "Call to mmap failed with error ENOMEM - no memory is
available");
> +          break;
> +        default:
> +          sprintf(errorString, "Call to mmap returned with errno %d", errno);
> +        }
> +      throwJavaIoIOException(env, errorString);
>        return -1;
>      }
>    return (jlong) ((IDATA)mapAddress);
> 
> Modified: harmony/enhanced/classlib/trunk/modules/nio/src/test/java/common/org/apache/harmony/nio/tests/java/nio/MappedByteBufferTest.java
> URL: http://svn.apache.org/viewvc/harmony/enhanced/classlib/trunk/modules/nio/src/test/java/common/org/apache/harmony/nio/tests/java/nio/MappedByteBufferTest.java?rev=815994&r1=815993&r2=815994&view=diff
> ==============================================================================
> --- harmony/enhanced/classlib/trunk/modules/nio/src/test/java/common/org/apache/harmony/nio/tests/java/nio/MappedByteBufferTest.java
(original)
> +++ harmony/enhanced/classlib/trunk/modules/nio/src/test/java/common/org/apache/harmony/nio/tests/java/nio/MappedByteBufferTest.java
Wed Sep 16 22:44:49 2009
> @@ -22,17 +22,19 @@
>  import java.io.FileOutputStream;
>  import java.io.IOException;
>  import java.io.RandomAccessFile;
> +import java.nio.BufferUnderflowException;
>  import java.nio.ByteBuffer;
>  import java.nio.IntBuffer;
>  import java.nio.MappedByteBuffer;
>  import java.nio.channels.FileChannel;
> +import java.nio.channels.NonWritableChannelException;
>  import java.nio.channels.FileChannel.MapMode;
>  
>  import junit.framework.TestCase;
>  
>  public class MappedByteBufferTest extends TestCase {
>  
> -    File tmpFile;
> +    File tmpFile, emptyFile;
>      
>      /**
>       * A regression test for failing to correctly set capacity of underlying
> @@ -63,6 +65,62 @@
>      }
>      
>      /**
> +     * Regression for HARMONY-6315 - FileChannel.map throws IOException
> +     * when called with size 0
> +     * 
> +     * @throws IOException
> +     */
> +    public void testEmptyBuffer() throws IOException {
> +    	// Map empty file
> +        FileInputStream fis = new FileInputStream(emptyFile);
> +        FileChannel fc = fis.getChannel();
> +        MappedByteBuffer mmb = fc.map(FileChannel.MapMode.READ_ONLY, 0, fc.size());
> +        
> +        // check non-null
> +        assertNotNull("MappedByteBuffer created from empty file should not be null",

> +        		mmb);
> +        
> +        // check capacity is 0
> +        int len = mmb.capacity();
> +        assertEquals("MappedByteBuffer created from empty file should have 0 capacity",

> +        		0, len);
> +        
> +        assertFalse("MappedByteBuffer from empty file shouldn't be backed by an array
", 
> +        		mmb.hasArray());
> +        
> +        try
> +        {
> +        	byte b = mmb.get();
> +        	fail("Calling MappedByteBuffer.get() on empty buffer should throw a BufferUnderflowException");
> +        }
> +        catch (BufferUnderflowException e)
> +        {
> +        	// expected behaviour
> +        }
> +        
> +        // test expected exceptions thrown
> +        try 
> +        {
> +        	mmb = fc.map(FileChannel.MapMode.READ_WRITE, 0, fc.size());
> +        	fail("Expected NonWritableChannelException to be thrown");
> +        }
> +        catch (NonWritableChannelException e)
> +        {
> +        	// expected behaviour
> +        }
> +        try
> +        {
> +        	mmb = fc.map(FileChannel.MapMode.PRIVATE, 0, fc.size());
> +        	fail("Expected NonWritableChannelException to be thrown");
> +        }
> +        catch (NonWritableChannelException e)
> +        {
> +        	// expected behaviour
> +        }
> +        fc.close();
> +    }
> +    
> +    /**
>       * @tests {@link java.nio.MappedByteBuffer#force()}
>       */
>      public void test_force() throws IOException {
> @@ -146,5 +204,8 @@
>          fileChannel.write(byteBuffer);
>          fileChannel.close();
>          fileOutputStream.close();
> +        
> +        emptyFile = File.createTempFile("harmony", "test");  //$NON-NLS-1$//$NON-NLS-2$
> +        emptyFile.deleteOnExit();
>      }
>  }
> 
> 
> 

Mime
View raw message