Removed redundant buffering code from FileReader and added a test.

io/CMakeLists.txt

@@ -17,4 +17,4 @@ target_link_libraries(cjdio cjdmemory ${LIBEVENT2_LIBRARIES})
 
 # Everything must be tested.
 enable_testing()
-#add_subdirectory(test) later
+add_subdirectory(test)

io/FileReader.c

@@ -12,118 +12,70 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 #include "io/Reader.h"
+#include "io/FileReader.h"
 #include "memory/Allocator.h"
 
 #include <string.h>
 #include <stdio.h>
+#include <stdint.h>
 #include <stdbool.h>
 
-#define BUFFER_SIZE 1024
-
 struct Context {
     FILE* toRead;
-    char buffer[BUFFER_SIZE];
-    char* pointer;
-    char* endPointer;
-    bool skipFailed;
+    bool failed;
     size_t bytesRead;
     struct Reader reader;
 };
 
-static int read(void* readInto, size_t length, const struct Reader* reader);
-static void skip(size_t byteCount, const struct Reader* reader);
-static size_t bytesRead(const struct Reader* reader);
-
-/** @see FileReader.h */
-struct Reader* FileReader_new(FILE* toRead, const struct Allocator* allocator)
-{
-    struct Context* context = allocator->calloc(sizeof(struct Context), 1, allocator);
-
-    context->toRead = toRead;
-    context->pointer = context->buffer;
-    context->endPointer = context->buffer + fread(context->buffer, 1, BUFFER_SIZE, toRead);
-
-    struct Reader localReader = {
-        .context = context,
-        .read = read,
-        .skip = skip,
-        .bytesRead = bytesRead
-    };
-    memcpy(&context->reader, &localReader, sizeof(struct Reader));
-
-    return &context->reader;
-}
-
 /** @see Reader->read() */
 static int read(void* readInto, size_t length, const struct Reader* reader)
 {
     struct Context* context = (struct Context*) reader->context;
-    context->bytesRead += length;
-
-    if (length == 0) {
-        int ret = read(readInto, 1, reader);
-        context->pointer--;
-        return ret;
-    }
-
-    if (context->pointer + length > context->endPointer) {
-        size_t firstRead = context->endPointer - context->pointer;
-        if (firstRead > 0) {
-            read(readInto, firstRead, reader);
-        }
-        if (length - firstRead >= BUFFER_SIZE) {
-            size_t r = fread((char*)readInto + firstRead, 1, length - firstRead, context->toRead);
-            if (r < length - firstRead) {
-                return -1;
-            }
-        }
-        context->pointer = context->buffer;
-        context->endPointer =
-            context->buffer + fread(context->buffer, 1, BUFFER_SIZE, context->toRead);
-        if (length - firstRead >= BUFFER_SIZE) {
-            return 0;
-        }
-        if (context->pointer == context->endPointer) {
-            // EOF
-            return -1;
-        }
-        if (length - firstRead < BUFFER_SIZE) {
-            return read((char*)readInto + firstRead, length - firstRead, reader);
-        }
+    if (context->failed || fread((char*)readInto, 1, length, context->toRead) != length) {
+        context->failed = true;
+        return -1;
     }
-
-    // Check for integer overflow
-    if (context->pointer + length < context->pointer) {
-        return -2;
-    }
-
-    memcpy(readInto, context->pointer, length);
-    context->pointer += length;
-
+    context->bytesRead += length;
     return 0;
 }
 
 /** @see Reader->bytesRead() */
 static size_t bytesRead(const struct Reader* reader)
 {
+    // ftell() is unreliable.
     return ((struct Context*) reader->context)->bytesRead;
 }
 
 /** @see Reader->skip() */
 static void skip(size_t length, const struct Reader* reader)
 {
     struct Context* context = (struct Context*) reader->context;
-    context->bytesRead += length;
 
-    while (context->pointer + length > context->endPointer) {
-        length -= (context->endPointer - context->pointer + 1);
-        char* discard;
-        context->pointer = context->endPointer;
-        if (read(&discard, 1, reader)) {
-            context->skipFailed = true;
-            return;
-        }
+    #define BUFF_SZ 256
+    uint8_t buff[BUFF_SZ];
+
+    // fseek() and ftell() are unreliable.
+    size_t amount;
+    while ((amount = (length > BUFF_SZ) ? BUFF_SZ : length) && !context->failed) {
+        context->failed = read(buff, amount, reader);
+        length -= amount;
     }
+}
+
+/** @see FileReader.h */
+struct Reader* FileReader_new(FILE* toRead, const struct Allocator* allocator)
+{
+    struct Context* context = allocator->calloc(sizeof(struct Context), 1, allocator);
+
+    context->toRead = toRead;
+
+    struct Reader localReader = {
+        .context = context,
+        .read = read,
+        .skip = skip,
+        .bytesRead = bytesRead
+    };
+    memcpy(&context->reader, &localReader, sizeof(struct Reader));
 
-    context->pointer += length;
+    return &context->reader;
 }

io/test/CMakeLists.txt

@@ -0,0 +1,30 @@
+# You may redistribute this program and/or modify it under the terms of
+# the GNU General Public License as published by the Free Software Foundation,
+# either version 3 of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+cmake_minimum_required(VERSION 2.4)
+
+string(REGEX REPLACE "^.*/" "" main_dir_name ${CMAKE_SOURCE_DIR})
+string(REPLACE ${CMAKE_SOURCE_DIR} ${main_dir_name} this_dir ${CMAKE_CURRENT_SOURCE_DIR})
+message("-- Tests to run for " ${this_dir})
+add_definitions(-g)
+
+# Anything in this dir which ends with "test.c" is considered a test.
+file(GLOB tests "*test.c")
+foreach(test_source_fullpath ${tests})
+  string(REGEX REPLACE "^.*/" "" test_source ${test_source_fullpath})
+  string(REPLACE "test.c" "test" test_bin ${test_source})
+  message("     " ${test_source})
+  add_executable(${test_bin} ${test_source})
+  target_link_libraries(${test_bin} crypto cjdmemory ${LIBEVENT2_LIBRARIES})
+  add_test(${test_bin} ${test_bin})
+endforeach()
+# Add an empty line after tests.
+message("")

io/test/FileReader_test.c

@@ -0,0 +1,61 @@
+/*
+ * You may redistribute this program and/or modify it under the terms of
+ * the GNU General Public License as published by the Free Software Foundation,
+ * either version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include "crypto/Crypto.h"
+#include "io/Reader.h"
+#include "io/FileReader.h"
+#include "memory/Allocator.h"
+#include "memory/MallocAllocator.h"
+
+#include <string.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+int main()
+{
+    FILE* tmp = tmpfile();
+    uint8_t buffer1[2048];
+    randombytes(buffer1, 2048);
+    fwrite(buffer1, 1, 2048, tmp);
+
+    struct Allocator* alloc = MallocAllocator_new(256);
+
+    uint8_t buffer2[1024];
+    rewind(tmp);
+    struct Reader* r = FileReader_new(tmp, alloc);
+
+    r->read(buffer2, 128, r);
+    r->skip(128, r);
+    r->read(buffer2+128, 128, r);
+    r->skip(512, r);
+    r->read(buffer2+128+128, 256, r);
+    r->skip(300, r);
+    r->read(buffer2+128+128+256, 128, r);
+
+    assert(r->bytesRead(r) == 128+128+128+512+256+300+128);
+
+    uint8_t* ptr1 = buffer1;
+    uint8_t* ptr2 = buffer2;
+
+    #define SKIP(x) ptr1 += x
+    #define CMP(x) assert(!memcmp(ptr1, ptr2, x)); ptr1 += x; ptr2 += x
+
+    CMP(128);
+    SKIP(128);
+    CMP(128);
+    SKIP(512);
+    CMP(256);
+    SKIP(300);
+    CMP(128);
+}