diff --git a/Subtrees/beast/modules/beast_core/files/beast_RandomAccessFile.cpp b/Subtrees/beast/modules/beast_core/files/beast_RandomAccessFile.cpp index 0131f520a0..8ca6843491 100644 --- a/Subtrees/beast/modules/beast_core/files/beast_RandomAccessFile.cpp +++ b/Subtrees/beast/modules/beast_core/files/beast_RandomAccessFile.cpp @@ -3,10 +3,6 @@ This file is part of Beast: https://github.com/vinniefalco/Beast Copyright 2013, Vinnie Falco - Portions of this file are from JUCE. - Copyright (c) 2013 - Raw Material Software Ltd. - Please visit http://www.juce.com - Permission to use, copy, modify, and/or distribute this software for any purpose with or without fee is hereby granted, provided that the above copyright notice and this permission notice appear in all copies. @@ -54,16 +50,17 @@ void RandomAccessFile::close () Result RandomAccessFile::setPosition (FileOffset newPosition) { - Result result (Result::ok ()); - if (newPosition != currentPosition) { flushBuffer (); - result = nativeSetPosition (newPosition); + // VFALCO NOTE I dislike return from the middle but + // Result::ok() is showing up in the profile + // + return nativeSetPosition (newPosition); } - return result; + return Result::ok (); } Result RandomAccessFile::read (void* buffer, ByteCount numBytes, ByteCount* pActualAmount) @@ -157,16 +154,13 @@ Result RandomAccessFile::flushBuffer () class RandomAccessFileTests : public UnitTest { public: - RandomAccessFileTests () - : UnitTest ("RandomAccessFile", "beast") - , numRecords (1000) - , seedValue (50) + RandomAccessFileTests () : UnitTest ("RandomAccessFile", "beast") { } /* For this test we will create a file which consists of a fixed number of variable length records. Each record is numbered sequentially - start at 1. To calculate the position of each record we first build + starting at 0. To calculate the position of each record we first build a table of size/offset pairs using a pseudorandom number generator. */ struct Record @@ -206,6 +200,8 @@ public: repeatableShuffle (numRecords, records, seedValue); } + // Write all the records to the file. + // The payload is pseudo-randomly generated. void writeRecords (RandomAccessFile& file, int numRecords, HeapBlock const& records, @@ -229,23 +225,26 @@ public: } } + // Read the records and verify the consistency. void readRecords (RandomAccessFile& file, int numRecords, - HeapBlock const & records, + HeapBlock const& records, int64 seedValue) { using namespace UnitTestUtilities; for (int i = 0; i < numRecords; ++i) { - int const bytes = records [i].bytes; + Record const& record (records [i]); + + int const bytes = record.bytes; Payload p1 (bytes); Payload p2 (bytes); - p1.repeatableRandomFill (bytes, bytes, records [i].index + seedValue); + p1.repeatableRandomFill (bytes, bytes, record.index + seedValue); - file.setPosition (records [i].offset); + file.setPosition (record.offset); Result result = file.read (p2.data.getData (), bytes); @@ -260,48 +259,68 @@ public: } } - void testFile (int const bufferSize) + // Perform the test at the given buffer size. + void testFile (int const numRecords, int const bufferSize) { using namespace UnitTestUtilities; - String s; - s << "bufferSize = " << String (bufferSize); - beginTest (s); + int const seedValue = 50; + + beginTest (String ("numRecords=") + String (numRecords) + ", bufferSize=" + String (bufferSize)); int const maxPayload = bmax (1000, bufferSize * 2); - RandomAccessFile file (bufferSize); + // Calculate the path + File const path (File::createTempFile ("RandomAccessFile")); - Result result = file.open (File::createTempFile ("tests"), RandomAccessFile::readWrite); + // Create a predictable set of records + HeapBlock records (numRecords); + createRecords (records, numRecords, maxPayload, seedValue); - expect (result.wasOk (), "Should be ok"); + Result result (Result::ok ()); + + { + // Create the file + RandomAccessFile file (bufferSize); + result = file.open (path, RandomAccessFile::readWrite); + expect (result.wasOk (), "Should be ok"); + + if (result.wasOk ()) + { + writeRecords (file, numRecords, records, seedValue); + + readRecords (file, numRecords, records, seedValue); + + repeatableShuffle (numRecords, records, seedValue); + + readRecords (file, numRecords, records, seedValue); + } + } if (result.wasOk ()) { - HeapBlock records (numRecords); + // Re-open the file in read only mode + RandomAccessFile file (bufferSize); + result = file.open (path, RandomAccessFile::readOnly); + expect (result.wasOk (), "Should be ok"); - createRecords (records, numRecords, maxPayload, seedValue); - - writeRecords (file, numRecords, records, seedValue); - - readRecords (file, numRecords, records, seedValue); - - repeatableShuffle (numRecords, records, seedValue); - - readRecords (file, numRecords, records, seedValue); + if (result.wasOk ()) + { + readRecords (file, numRecords, records, seedValue); + } } } void runTest () { - testFile (0); - testFile (1000); - testFile (10000); + int const numRecords = 1000; + + testFile (numRecords, 0); + testFile (numRecords, 1000); + testFile (numRecords, 10000); } private: - int const numRecords; - int64 const seedValue; }; static RandomAccessFileTests randomAccessFileTests; diff --git a/Subtrees/beast/modules/beast_core/files/beast_RandomAccessFile.h b/Subtrees/beast/modules/beast_core/files/beast_RandomAccessFile.h index b97e17dcb1..f2eaca1ccf 100644 --- a/Subtrees/beast/modules/beast_core/files/beast_RandomAccessFile.h +++ b/Subtrees/beast/modules/beast_core/files/beast_RandomAccessFile.h @@ -41,6 +41,9 @@ @note All files are opened in binary mode. No text newline conversions are performed. + @note None of these members are thread safe. The caller is responsible + for synchronization. + @see FileInputStream, FileOutputStream */ class BEAST_API RandomAccessFile : Uncopyable, LeakChecked @@ -140,7 +143,7 @@ public: by `buffer` is at least as large as `bytesToRead`. @note The file must have been opened with read permission. - + @param buffer The memory to store the incoming data @param numBytes The number of bytes to read. @param pActualAmount Pointer to store the actual amount read, or `nullptr`. @@ -212,7 +215,7 @@ public: int read (void* destBuffer, int maxBytesToRead) { size_t actualBytes = 0; - m_file.read (destBuffer, maxBytesToRead, &actualBytes); + m_file.read (destBuffer, maxBytesToRead, &actualBytes); return actualBytes; } diff --git a/Subtrees/beast/modules/beast_core/native/beast_posix_SharedCode.h b/Subtrees/beast/modules/beast_core/native/beast_posix_SharedCode.h index 622d48a141..222490176e 100644 --- a/Subtrees/beast/modules/beast_core/native/beast_posix_SharedCode.h +++ b/Subtrees/beast/modules/beast_core/native/beast_posix_SharedCode.h @@ -586,54 +586,70 @@ Result RandomAccessFile::nativeSetPosition (FileOffset newPosition) { bassert (isOpen ()); - Result result (Result::ok ()); + off_t const actualPosition = lseek (getFD (fileHandle), newPosition, SEEK_SET); - off_t const actual = lseek (getFD (fileHandle), newPosition, SEEK_SET); + currentPosition = actualPosition; - if (actual != newPosition) - result = getResultForErrno(); + if (actualPosition != newPosition) + { + // VFALCO NOTE I dislike return from the middle but + // Result::ok() is showing up in the profile + // + return getResultForErrno(); + } - return result; + return Result::ok(); } Result RandomAccessFile::nativeRead (void* buffer, ByteCount numBytes, ByteCount* pActualAmount) { bassert (isOpen ()); - Result result (Result::ok ()); + ssize_t bytesRead = ::read (getFD (fileHandle), buffer, numBytes); - ssize_t amount = ::read (getFD (fileHandle), buffer, numBytes); - - if (amount < 0) + if (bytesRead < 0) { - result = getResultForErrno(); - amount = 0; + if (pActualAmount != nullptr) + *pActualAmount = 0; + + // VFALCO NOTE I dislike return from the middle but + // Result::ok() is showing up in the profile + // + return getResultForErrno(); } - if (pActualAmount != nullptr) - *pActualAmount = amount; + currentPosition += bytesRead; - return result; + if (pActualAmount != nullptr) + *pActualAmount = bytesRead; + + return Result::ok(); } Result RandomAccessFile::nativeWrite (void const* data, ByteCount numBytes, size_t* pActualAmount) { bassert (isOpen ()); - Result result (Result::ok ()); + ssize_t bytesWritten = ::write (getFD (fileHandle), data, numBytes); - ssize_t actual = ::write (getFD (fileHandle), data, numBytes); - - if (actual == -1) + // write(3) says that the actual return will be exactly -1 on + // error, but we will assume anything negative indicates failure. + // + if (bytesWritten < 0) { - result = getResultForErrno(); - actual = 0; + if (pActualAmount != nullptr) + *pActualAmount = 0; + + // VFALCO NOTE I dislike return from the middle but + // Result::ok() is showing up in the profile + // + return getResultForErrno(); } if (pActualAmount != nullptr) - *pActualAmount = actual; + *pActualAmount = bytesWritten; - return result; + return Result::ok(); } Result RandomAccessFile::nativeTruncate () @@ -654,14 +670,14 @@ Result RandomAccessFile::nativeFlush () if (fsync (getFD (fileHandle)) == -1) result = getResultForErrno(); - #if BEAST_ANDROID +#if BEAST_ANDROID // This stuff tells the OS to asynchronously update the metadata // that the OS has cached aboud the file - this metadata is used // when the device is acting as a USB drive, and unless it's explicitly // refreshed, it'll get out of step with the real file. const LocalRef t (javaString (file.getFullPathName())); android.activity.callVoidMethod (BeastAppActivity.scanFile, t.get()); - #endif +#endif return result; }