Browse Source

rwops: Make read and write work like POSIX, not stdio.

This simplifies some things, clarifies some things, and also allows
for the possibility of RWops that offer non-blocking i/o (although
none of the current built-in ones do, intentionally, we could add this
later if we choose, or people could provide things like network socket
RWops implementations now, etc.

Fixes #6729.
Ryan C. Gordon 2 years ago
parent
commit
72c1f73bc5

+ 53 - 33
include/SDL3/SDL_rwops.h

@@ -66,22 +66,32 @@ typedef struct SDL_RWops
                              int whence);
 
     /**
-     *  Read up to \c maxnum objects each of size \c size from the data
-     *  stream to the area pointed at by \c ptr.
+     *  Read up to \c size bytes from the data stream to the area pointed
+     *  at by \c ptr.
      *
-     *  \return the number of objects read, or 0 at error or end of file.
+     *  It is an error to use a negative \c size, but this parameter is
+     *  signed so you definitely cannot overflow the return value on a
+     *  successful run with enormous amounts of data.
+     *
+     *  \return the number of objects read, or 0 on end of file, or -1 on error.
      */
-    size_t (SDLCALL * read) (struct SDL_RWops * context, void *ptr,
-                             size_t size, size_t maxnum);
+    Sint64 (SDLCALL * read) (struct SDL_RWops * context, void *ptr,
+                             Sint64 size);
 
     /**
-     *  Write exactly \c num objects each of size \c size from the area
-     *  pointed at by \c ptr to data stream.
+     *  Write exactly \c size bytes from the area pointed at by \c ptr
+     *  to data stream. May write less than requested (error, non-blocking i/o,
+     *  etc). Returns -1 on error when nothing was written.
+     *
+     *  It is an error to use a negative \c size, but this parameter is
+     *  signed so you definitely cannot overflow the return value on a
+     *  successful run with enormous amounts of data.
      *
-     *  \return the number of objects written, or 0 at error or end of file.
+     *  \return the number of bytes written, which might be less than \c size,
+     *          and -1 on error.
      */
-    size_t (SDLCALL * write) (struct SDL_RWops * context, const void *ptr,
-                              size_t size, size_t num);
+    Sint64 (SDLCALL * write) (struct SDL_RWops * context, const void *ptr,
+                              Sint64 size);
 
     /**
      *  Close and free an allocated SDL_RWops structure.
@@ -406,22 +416,25 @@ extern DECLSPEC Sint64 SDLCALL SDL_RWtell(SDL_RWops *context);
 /**
  * Read from a data source.
  *
- * This function reads up to `maxnum` objects each of size `size` from the
- * data source to the area pointed at by `ptr`. This function may read less
- * objects than requested. It will return zero when there has been an error or
- * the data stream is completely read.
+ * This function reads up `size` bytes from the data source to the area
+ * pointed at by `ptr`. This function may read less bytes than requested.
+ * It will return zero when the data stream is completely read, or
+ * -1 on error. For streams that support non-blocking
+ * operation, if nothing was read because it would require blocking,
+ * this function returns -2 to distinguish that this is not an error or
+ * end-of-file, and the caller can try again later.
  *
  * SDL_RWread() is actually a function wrapper that calls the SDL_RWops's
  * `read` method appropriately, to simplify application development.
  *
- * Prior to SDL 2.0.10, this function was a macro.
+ * It is an error to specify a negative `size`, but this parameter is
+ * signed so you definitely cannot overflow the return value on a
+ * successful run with enormous amounts of data.
  *
  * \param context a pointer to an SDL_RWops structure
  * \param ptr a pointer to a buffer to read data into
- * \param size the size of each object to read, in bytes
- * \param maxnum the maximum number of objects to be read
- * \returns the number of objects read, or 0 at error or end of file; call
- *          SDL_GetError() for more information.
+ * \param size the number of bytes to read from the data source.
+ * \returns the number of bytes read, or 0 at end of file, or -1 on error.
  *
  * \since This function is available since SDL 3.0.0.
  *
@@ -432,28 +445,36 @@ extern DECLSPEC Sint64 SDLCALL SDL_RWtell(SDL_RWops *context);
  * \sa SDL_RWseek
  * \sa SDL_RWwrite
  */
-extern DECLSPEC size_t SDLCALL SDL_RWread(SDL_RWops *context,
-                                          void *ptr, size_t size,
-                                          size_t maxnum);
+extern DECLSPEC Sint64 SDLCALL SDL_RWread(SDL_RWops *context,
+                                          void *ptr, Sint64 size);
 
 /**
  * Write to an SDL_RWops data stream.
  *
- * This function writes exactly `num` objects each of size `size` from the
- * area pointed at by `ptr` to the stream. If this fails for any reason, it'll
- * return less than `num` to demonstrate how far the write progressed. On
- * success, it returns `num`.
+ * This function writes exactly `size` bytes from the area pointed at by
+ * `ptr` to the stream. If this fails for any reason, it'll return less
+ * than `size` to demonstrate how far the write progressed. On success,
+ * it returns `num`.
+ *
+ * On error, this function still attempts to write as much as possible,
+ * so it might return a positive value less than the requested write
+ * size. If the function failed to write anything and there was an
+ * actual error, it will return -1. For streams that support non-blocking
+ * operation, if nothing was written because it would require blocking,
+ * this function returns -2 to distinguish that this is not an error and
+ * the caller can try again later.
  *
  * SDL_RWwrite is actually a function wrapper that calls the SDL_RWops's
  * `write` method appropriately, to simplify application development.
  *
- * Prior to SDL 2.0.10, this function was a macro.
+ * It is an error to specify a negative `size`, but this parameter is
+ * signed so you definitely cannot overflow the return value on a
+ * successful run with enormous amounts of data.
  *
  * \param context a pointer to an SDL_RWops structure
  * \param ptr a pointer to a buffer containing data to write
- * \param size the size of an object to write, in bytes
- * \param num the number of objects to write
- * \returns the number of objects written, which will be less than **num** on
+ * \param size the number of bytes to write
+ * \returns the number of bytes written, which will be less than `num` on
  *          error; call SDL_GetError() for more information.
  *
  * \since This function is available since SDL 3.0.0.
@@ -465,9 +486,8 @@ extern DECLSPEC size_t SDLCALL SDL_RWread(SDL_RWops *context,
  * \sa SDL_RWread
  * \sa SDL_RWseek
  */
-extern DECLSPEC size_t SDLCALL SDL_RWwrite(SDL_RWops *context,
-                                           const void *ptr, size_t size,
-                                           size_t num);
+extern DECLSPEC Sint64 SDLCALL SDL_RWwrite(SDL_RWops *context,
+                                           const void *ptr, Sint64 size);
 
 /**
  * Close and free an allocated SDL_RWops structure.

+ 6 - 6
src/audio/SDL_wave.c

@@ -1523,7 +1523,7 @@ static int WaveNextChunk(SDL_RWops *src, WaveChunk *chunk)
     if (SDL_RWseek(src, nextposition, RW_SEEK_SET) != nextposition) {
         /* Not sure how we ended up here. Just abort. */
         return -2;
-    } else if (SDL_RWread(src, chunkheader, 4, 2) != 2) {
+    } else if (SDL_RWread(src, chunkheader, sizeof(Uint32) * 2) != (sizeof(Uint32) * 2)) {
         return -1;
     }
 
@@ -1553,7 +1553,7 @@ static int WaveReadPartialChunkData(SDL_RWops *src, WaveChunk *chunk, size_t len
             return -2;
         }
 
-        chunk->size = SDL_RWread(src, chunk->data, 1, length);
+        chunk->size = SDL_RWread(src, chunk->data, length);
         if (chunk->size != length) {
             /* Expected to be handled by the caller. */
         }
@@ -1650,7 +1650,7 @@ static int WaveReadFormat(WaveFile *file)
         format->validsamplebits = SDL_ReadLE16(fmtsrc);
         format->samplesperblock = format->validsamplebits;
         format->channelmask = SDL_ReadLE32(fmtsrc);
-        SDL_RWread(fmtsrc, format->subformat, 1, 16);
+        SDL_RWread(fmtsrc, format->subformat, 16);
         format->encoding = WaveGetFormatGUIDEncoding(format);
     }
 
@@ -1806,7 +1806,7 @@ static int WaveLoad(SDL_RWops *src, WaveFile *file, SDL_AudioSpec *spec, Uint8 *
     if (RIFFchunk.fourcc == RIFF) {
         Uint32 formtype;
         /* Read the form type. "WAVE" expected. */
-        if (SDL_RWread(src, &formtype, sizeof(Uint32), 1) != 1) {
+        if (SDL_RWread(src, &formtype, sizeof(Uint32)) != sizeof (Uint32)) {
             return SDL_SetError("Could not read RIFF form type");
         } else if (SDL_SwapLE32(formtype) != WAVE) {
             return SDL_SetError("RIFF form type is not WAVE (not a Waveform file)");
@@ -1896,7 +1896,7 @@ static int WaveLoad(SDL_RWops *src, WaveFile *file, SDL_AudioSpec *spec, Uint8 *
                     /* Let's use src directly, it's just too convenient. */
                     Sint64 position = SDL_RWseek(src, chunk->position, RW_SEEK_SET);
                     Uint32 samplelength;
-                    if (position == chunk->position && SDL_RWread(src, &samplelength, sizeof(Uint32), 1) == 1) {
+                    if (position == chunk->position && SDL_RWread(src, &samplelength, sizeof(Uint32)) == sizeof(Uint32)) {
                         file->fact.status = 1;
                         file->fact.samplelength = SDL_SwapLE32(samplelength);
                     } else {
@@ -1941,7 +1941,7 @@ static int WaveLoad(SDL_RWops *src, WaveFile *file, SDL_AudioSpec *spec, Uint8 *
             Uint64 position = (Uint64)chunk->position + chunk->length - 1;
             if (position > SDL_MAX_SINT64 || SDL_RWseek(src, (Sint64)position, RW_SEEK_SET) != (Sint64)position) {
                 return SDL_SetError("Could not seek to WAVE chunk data");
-            } else if (SDL_RWread(src, &tmp, 1, 1) != 1) {
+            } else if (SDL_RWread(src, &tmp, 1) != 1) {
                 return SDL_SetError("RIFF size truncates chunk");
             }
         }

+ 5 - 5
src/audio/disk/SDL_diskaudio.c

@@ -47,16 +47,16 @@ static void DISKAUDIO_WaitDevice(_THIS)
 
 static void DISKAUDIO_PlayDevice(_THIS)
 {
-    const size_t written = SDL_RWwrite(_this->hidden->io,
+    const Sint64 written = SDL_RWwrite(_this->hidden->io,
                                        _this->hidden->mixbuf,
-                                       1, _this->spec.size);
+                                       _this->spec.size);
 
     /* If we couldn't write, assume fatal error for now */
     if (written != _this->spec.size) {
         SDL_OpenedAudioDeviceDisconnected(_this);
     }
 #ifdef DEBUG_AUDIO
-    fprintf(stderr, "Wrote %d bytes of audio data\n", written);
+    fprintf(stderr, "Wrote %d bytes of audio data\n", (int) written);
 #endif
 }
 
@@ -73,8 +73,8 @@ static int DISKAUDIO_CaptureFromDevice(_THIS, void *buffer, int buflen)
     SDL_Delay(h->io_delay);
 
     if (h->io) {
-        const size_t br = SDL_RWread(h->io, buffer, 1, buflen);
-        buflen -= (int)br;
+        const int br = (int) SDL_RWread(h->io, buffer, (Sint64) buflen);
+        buflen -= br;
         buffer = ((Uint8 *)buffer) + br;
         if (buflen > 0) { /* EOF (or error, but whatever). */
             SDL_RWclose(h->io);

+ 4 - 16
src/core/android/SDL_android.c

@@ -1846,27 +1846,15 @@ int Android_JNI_FileOpen(SDL_RWops *ctx,
     return 0;
 }
 
-size_t Android_JNI_FileRead(SDL_RWops *ctx, void *buffer,
-                            size_t size, size_t maxnum)
+Sint64 Android_JNI_FileRead(SDL_RWops *ctx, void *buffer, Sint64 size)
 {
-    size_t result;
     AAsset *asset = (AAsset *)ctx->hidden.androidio.asset;
-    result = AAsset_read(asset, buffer, size * maxnum);
-
-    if (result > 0) {
-        /* Number of chuncks */
-        return result / size;
-    } else {
-        /* Error or EOF */
-        return result;
-    }
+    return (Sint64) AAsset_read(asset, buffer, (size_t) size);
 }
 
-size_t Android_JNI_FileWrite(SDL_RWops *ctx, const void *buffer,
-                             size_t size, size_t num)
+Sint64 Android_JNI_FileWrite(SDL_RWops *ctx, const void *buffer, Sint64 size)
 {
-    SDL_SetError("Cannot write to Android package filesystem");
-    return 0;
+    return SDL_SetError("Cannot write to Android package filesystem");
 }
 
 Sint64 Android_JNI_FileSize(SDL_RWops *ctx)

+ 2 - 2
src/core/android/SDL_android.h

@@ -62,8 +62,8 @@ extern SDL_bool Android_IsChromebook(void);
 int Android_JNI_FileOpen(SDL_RWops *ctx, const char *fileName, const char *mode);
 Sint64 Android_JNI_FileSize(SDL_RWops *ctx);
 Sint64 Android_JNI_FileSeek(SDL_RWops *ctx, Sint64 offset, int whence);
-size_t Android_JNI_FileRead(SDL_RWops *ctx, void *buffer, size_t size, size_t maxnum);
-size_t Android_JNI_FileWrite(SDL_RWops *ctx, const void *buffer, size_t size, size_t num);
+Sint64 Android_JNI_FileRead(SDL_RWops *ctx, void *buffer, Sint64 size);
+Sint64 Android_JNI_FileWrite(SDL_RWops *ctx, const void *buffer, Sint64 size);
 int Android_JNI_FileClose(SDL_RWops *ctx);
 
 /* Environment support */

+ 2 - 2
src/dynapi/SDL_dynapi_procs.h

@@ -753,8 +753,8 @@ SDL_DYNAPI_PROC(void,SDL_SIMDFree,(void *a),(a),)
 SDL_DYNAPI_PROC(Sint64,SDL_RWsize,(SDL_RWops *a),(a),return)
 SDL_DYNAPI_PROC(Sint64,SDL_RWseek,(SDL_RWops *a, Sint64 b, int c),(a,b,c),return)
 SDL_DYNAPI_PROC(Sint64,SDL_RWtell,(SDL_RWops *a),(a),return)
-SDL_DYNAPI_PROC(size_t,SDL_RWread,(SDL_RWops *a, void *b, size_t c, size_t d),(a,b,c,d),return)
-SDL_DYNAPI_PROC(size_t,SDL_RWwrite,(SDL_RWops *a, const void *b, size_t c, size_t d),(a,b,c,d),return)
+SDL_DYNAPI_PROC(Sint64,SDL_RWread,(SDL_RWops *a, void *b, Sint64 c),(a,b,c),return)
+SDL_DYNAPI_PROC(Sint64,SDL_RWwrite,(SDL_RWops *a, const void *b, Sint64 c),(a,b,c),return)
 SDL_DYNAPI_PROC(int,SDL_RWclose,(SDL_RWops *a),(a),return)
 SDL_DYNAPI_PROC(void*,SDL_LoadFile,(const char *a, size_t *b),(a,b),return)
 SDL_DYNAPI_PROC(SDL_MetalView,SDL_Metal_CreateView,(SDL_Window *a),(a),return)

+ 72 - 90
src/file/SDL_rwops.c

@@ -182,20 +182,21 @@ static Sint64 SDLCALL windows_file_seek(SDL_RWops *context, Sint64 offset, int w
     return windowsoffset.QuadPart;
 }
 
-static size_t SDLCALL
-windows_file_read(SDL_RWops *context, void *ptr, size_t size, size_t maxnum)
+static Sint64 SDLCALL
+windows_file_read(SDL_RWops *context, void *ptr, Sint64 size)
 {
-    size_t total_need;
+    const size_t total_need = (size_t) size;
     size_t total_read = 0;
     size_t read_ahead;
     DWORD byte_read;
 
-    total_need = size * maxnum;
-
-    if (context == NULL || context->hidden.windowsio.h == INVALID_HANDLE_VALUE || !total_need) {
+    if (context == NULL || context->hidden.windowsio.h == INVALID_HANDLE_VALUE) {
+        return SDL_SetError("Invalid file handle");
+    } else if (!total_need) {
         return 0;
     }
 
+
     if (context->hidden.windowsio.buffer.left > 0) {
         void *data = (char *)context->hidden.windowsio.buffer.data +
                      context->hidden.windowsio.buffer.size -
@@ -206,7 +207,7 @@ windows_file_read(SDL_RWops *context, void *ptr, size_t size, size_t maxnum)
         context->hidden.windowsio.buffer.left -= read_ahead;
 
         if (read_ahead == total_need) {
-            return maxnum;
+            return size;
         }
         ptr = (char *)ptr + read_ahead;
         total_need -= read_ahead;
@@ -216,8 +217,7 @@ windows_file_read(SDL_RWops *context, void *ptr, size_t size, size_t maxnum)
     if (total_need < READAHEAD_BUFFER_SIZE) {
         if (!ReadFile(context->hidden.windowsio.h, context->hidden.windowsio.buffer.data,
                       READAHEAD_BUFFER_SIZE, &byte_read, NULL)) {
-            SDL_Error(SDL_EFREAD);
-            return 0;
+            return SDL_Error(SDL_EFREAD);
         }
         read_ahead = SDL_min(total_need, (int)byte_read);
         SDL_memcpy(ptr, context->hidden.windowsio.buffer.data, read_ahead);
@@ -226,26 +226,23 @@ windows_file_read(SDL_RWops *context, void *ptr, size_t size, size_t maxnum)
         total_read += read_ahead;
     } else {
         if (!ReadFile(context->hidden.windowsio.h, ptr, (DWORD)total_need, &byte_read, NULL)) {
-            SDL_Error(SDL_EFREAD);
-            return 0;
+            return SDL_Error(SDL_EFREAD);
         }
         total_read += byte_read;
     }
-    return total_read / size;
+    return total_read;
 }
 
-static size_t SDLCALL
-windows_file_write(SDL_RWops *context, const void *ptr, size_t size,
-                   size_t num)
+static Sint64 SDLCALL
+windows_file_write(SDL_RWops *context, const void *ptr, Sint64 size)
 {
-
-    size_t total_bytes;
+    const size_t total_bytes = (size_t) size;
     DWORD byte_written;
     size_t nwritten;
 
-    total_bytes = size * num;
-
-    if (context == NULL || context->hidden.windowsio.h == INVALID_HANDLE_VALUE || !size || !total_bytes) {
+    if (context == NULL || context->hidden.windowsio.h == INVALID_HANDLE_VALUE) {
+        return SDL_SetError("Invalid file handle");
+    } else if (!total_bytes) {
         return 0;
     }
 
@@ -260,23 +257,19 @@ windows_file_write(SDL_RWops *context, const void *ptr, size_t size,
     if (context->hidden.windowsio.append) {
         if (SetFilePointer(context->hidden.windowsio.h, 0L, NULL, FILE_END) ==
             INVALID_SET_FILE_POINTER) {
-            SDL_Error(SDL_EFWRITE);
-            return 0;
+            return SDL_Error(SDL_EFWRITE);
         }
     }
 
     if (!WriteFile(context->hidden.windowsio.h, ptr, (DWORD)total_bytes, &byte_written, NULL)) {
-        SDL_Error(SDL_EFWRITE);
-        return 0;
+        return SDL_Error(SDL_EFWRITE);
     }
 
-    nwritten = byte_written / size;
-    return nwritten;
+    return (Sint64) byte_written;
 }
 
 static int SDLCALL windows_file_close(SDL_RWops *context)
 {
-
     if (context) {
         if (context->hidden.windowsio.h != INVALID_HANDLE_VALUE) {
             CloseHandle(context->hidden.windowsio.h);
@@ -378,28 +371,28 @@ static Sint64 SDLCALL stdio_seek(SDL_RWops *context, Sint64 offset, int whence)
     return SDL_Error(SDL_EFSEEK);
 }
 
-static size_t SDLCALL
-stdio_read(SDL_RWops *context, void *ptr, size_t size, size_t maxnum)
+static Sint64 SDLCALL
+stdio_read(SDL_RWops *context, void *ptr, Sint64 size)
 {
     size_t nread;
 
-    nread = fread(ptr, size, maxnum, (FILE *)context->hidden.stdio.fp);
+    nread = fread(ptr, 1, size, (FILE *)context->hidden.stdio.fp);
     if (nread == 0 && ferror((FILE *)context->hidden.stdio.fp)) {
-        SDL_Error(SDL_EFREAD);
+        return SDL_Error(SDL_EFREAD);
     }
-    return nread;
+    return (Sint64) nread;
 }
 
-static size_t SDLCALL
-stdio_write(SDL_RWops *context, const void *ptr, size_t size, size_t num)
+static Sint64 SDLCALL
+stdio_write(SDL_RWops *context, const void *ptr, Sint64 size)
 {
     size_t nwrote;
 
-    nwrote = fwrite(ptr, size, num, (FILE *)context->hidden.stdio.fp);
+    nwrote = fwrite(ptr, 1, size, (FILE *)context->hidden.stdio.fp);
     if (nwrote == 0 && ferror((FILE *)context->hidden.stdio.fp)) {
-        SDL_Error(SDL_EFWRITE);
+        return SDL_Error(SDL_EFWRITE);
     }
-    return nwrote;
+    return (Sint64) nwrote;
 }
 
 static int SDLCALL stdio_close(SDL_RWops *context)
@@ -469,44 +462,33 @@ static Sint64 SDLCALL mem_seek(SDL_RWops *context, Sint64 offset, int whence)
     return (Sint64)(context->hidden.mem.here - context->hidden.mem.base);
 }
 
-static size_t SDLCALL
-mem_read(SDL_RWops *context, void *ptr, size_t size, size_t maxnum)
+static Sint64 mem_io(SDL_RWops *context, void *dst, const void *src, Sint64 size)
 {
-    size_t total_bytes;
-    size_t mem_available;
-
-    total_bytes = (maxnum * size);
-    if (!maxnum || !size || ((total_bytes / maxnum) != size)) {
-        return 0;
-    }
-
-    mem_available = (context->hidden.mem.stop - context->hidden.mem.here);
-    if (total_bytes > mem_available) {
-        total_bytes = mem_available;
+    const Sint64 mem_available = (Sint64) (context->hidden.mem.stop - context->hidden.mem.here);
+    if (size > mem_available) {
+        size = mem_available;
     }
+    SDL_memcpy(dst, src, (size_t) size);
+    context->hidden.mem.here += size;
+    return size;
+}
 
-    SDL_memcpy(ptr, context->hidden.mem.here, total_bytes);
-    context->hidden.mem.here += total_bytes;
-
-    return total_bytes / size;
+static Sint64 SDLCALL
+mem_read(SDL_RWops *context, void *ptr, Sint64 size)
+{
+    return mem_io(context, ptr, context->hidden.mem.here, size);
 }
 
-static size_t SDLCALL
-mem_write(SDL_RWops *context, const void *ptr, size_t size, size_t num)
+static Sint64 SDLCALL
+mem_write(SDL_RWops *context, const void *ptr, Sint64 size)
 {
-    if ((context->hidden.mem.here + (num * size)) > context->hidden.mem.stop) {
-        num = (context->hidden.mem.stop - context->hidden.mem.here) / size;
-    }
-    SDL_memcpy(context->hidden.mem.here, ptr, num * size);
-    context->hidden.mem.here += num * size;
-    return num;
+    return mem_io(context, context->hidden.mem.here, ptr, size);
 }
 
-static size_t SDLCALL
-mem_writeconst(SDL_RWops *context, const void *ptr, size_t size, size_t num)
+static Sint64 SDLCALL
+mem_writeconst(SDL_RWops *context, const void *ptr, Sint64 size)
 {
-    SDL_SetError("Can't write to read-only memory");
-    return 0;
+    return SDL_SetError("Can't write to read-only memory");
 }
 
 static int SDLCALL mem_close(SDL_RWops *context)
@@ -693,7 +675,7 @@ SDL_LoadFile_RW(SDL_RWops *src, size_t *datasize, int freesrc)
 {
     static const Sint64 FILE_CHUNK_SIZE = 1024;
     Sint64 size;
-    size_t size_read, size_total;
+    Sint64 size_read, size_total;
     void *data = NULL, *newdata;
 
     if (src == NULL) {
@@ -709,7 +691,7 @@ SDL_LoadFile_RW(SDL_RWops *src, size_t *datasize, int freesrc)
 
     size_total = 0;
     for (;;) {
-        if ((((Sint64)size_total) + FILE_CHUNK_SIZE) > size) {
+        if ((size_total + FILE_CHUNK_SIZE) > size) {
             size = (size_total + FILE_CHUNK_SIZE);
             newdata = SDL_realloc(data, (size_t)(size + 1));
             if (newdata == NULL) {
@@ -721,7 +703,7 @@ SDL_LoadFile_RW(SDL_RWops *src, size_t *datasize, int freesrc)
             data = newdata;
         }
 
-        size_read = SDL_RWread(src, (char *)data + size_total, 1, (size_t)(size - size_total));
+        size_read = SDL_RWread(src, (char *)data + size_total, size - size_total);
         if (size_read == 0) {
             break;
         }
@@ -729,7 +711,7 @@ SDL_LoadFile_RW(SDL_RWops *src, size_t *datasize, int freesrc)
     }
 
     if (datasize) {
-        *datasize = size_total;
+        *datasize = (size_t) size_total;
     }
     ((char *)data)[size_total] = '\0';
 
@@ -764,16 +746,16 @@ SDL_RWtell(SDL_RWops *context)
     return context->seek(context, 0, RW_SEEK_CUR);
 }
 
-size_t
-SDL_RWread(SDL_RWops *context, void *ptr, size_t size, size_t maxnum)
+Sint64
+SDL_RWread(SDL_RWops *context, void *ptr, Sint64 size)
 {
-    return context->read(context, ptr, size, maxnum);
+    return context->read(context, ptr, size);
 }
 
-size_t
-SDL_RWwrite(SDL_RWops *context, const void *ptr, size_t size, size_t num)
+Sint64
+SDL_RWwrite(SDL_RWops *context, const void *ptr, Sint64 size)
 {
-    return context->write(context, ptr, size, num);
+    return context->write(context, ptr, size);
 }
 
 int SDL_RWclose(SDL_RWops *context)
@@ -787,7 +769,7 @@ Uint8 SDL_ReadU8(SDL_RWops *src)
 {
     Uint8 value = 0;
 
-    SDL_RWread(src, &value, sizeof(value), 1);
+    SDL_RWread(src, &value, sizeof(value));
     return value;
 }
 
@@ -796,7 +778,7 @@ SDL_ReadLE16(SDL_RWops *src)
 {
     Uint16 value = 0;
 
-    SDL_RWread(src, &value, sizeof(value), 1);
+    SDL_RWread(src, &value, sizeof(value));
     return SDL_SwapLE16(value);
 }
 
@@ -805,7 +787,7 @@ SDL_ReadBE16(SDL_RWops *src)
 {
     Uint16 value = 0;
 
-    SDL_RWread(src, &value, sizeof(value), 1);
+    SDL_RWread(src, &value, sizeof(value));
     return SDL_SwapBE16(value);
 }
 
@@ -814,7 +796,7 @@ SDL_ReadLE32(SDL_RWops *src)
 {
     Uint32 value = 0;
 
-    SDL_RWread(src, &value, sizeof(value), 1);
+    SDL_RWread(src, &value, sizeof(value));
     return SDL_SwapLE32(value);
 }
 
@@ -823,7 +805,7 @@ SDL_ReadBE32(SDL_RWops *src)
 {
     Uint32 value = 0;
 
-    SDL_RWread(src, &value, sizeof(value), 1);
+    SDL_RWread(src, &value, sizeof(value));
     return SDL_SwapBE32(value);
 }
 
@@ -832,7 +814,7 @@ SDL_ReadLE64(SDL_RWops *src)
 {
     Uint64 value = 0;
 
-    SDL_RWread(src, &value, sizeof(value), 1);
+    SDL_RWread(src, &value, sizeof(value));
     return SDL_SwapLE64(value);
 }
 
@@ -841,56 +823,56 @@ SDL_ReadBE64(SDL_RWops *src)
 {
     Uint64 value = 0;
 
-    SDL_RWread(src, &value, sizeof(value), 1);
+    SDL_RWread(src, &value, sizeof(value));
     return SDL_SwapBE64(value);
 }
 
 size_t
 SDL_WriteU8(SDL_RWops *dst, Uint8 value)
 {
-    return SDL_RWwrite(dst, &value, sizeof(value), 1);
+    return (SDL_RWwrite(dst, &value, sizeof(value)) == sizeof(value)) ? 1 : 0;
 }
 
 size_t
 SDL_WriteLE16(SDL_RWops *dst, Uint16 value)
 {
     const Uint16 swapped = SDL_SwapLE16(value);
-    return SDL_RWwrite(dst, &swapped, sizeof(swapped), 1);
+    return (SDL_RWwrite(dst, &swapped, sizeof(swapped)) == sizeof(swapped)) ? 1 : 0;
 }
 
 size_t
 SDL_WriteBE16(SDL_RWops *dst, Uint16 value)
 {
     const Uint16 swapped = SDL_SwapBE16(value);
-    return SDL_RWwrite(dst, &swapped, sizeof(swapped), 1);
+    return (SDL_RWwrite(dst, &swapped, sizeof(swapped)) == sizeof(swapped)) ? 1 : 0;
 }
 
 size_t
 SDL_WriteLE32(SDL_RWops *dst, Uint32 value)
 {
     const Uint32 swapped = SDL_SwapLE32(value);
-    return SDL_RWwrite(dst, &swapped, sizeof(swapped), 1);
+    return (SDL_RWwrite(dst, &swapped, sizeof(swapped)) == sizeof(swapped)) ? 1 : 0;
 }
 
 size_t
 SDL_WriteBE32(SDL_RWops *dst, Uint32 value)
 {
     const Uint32 swapped = SDL_SwapBE32(value);
-    return SDL_RWwrite(dst, &swapped, sizeof(swapped), 1);
+    return (SDL_RWwrite(dst, &swapped, sizeof(swapped)) == sizeof(swapped)) ? 1 : 0;
 }
 
 size_t
 SDL_WriteLE64(SDL_RWops *dst, Uint64 value)
 {
     const Uint64 swapped = SDL_SwapLE64(value);
-    return SDL_RWwrite(dst, &swapped, sizeof(swapped), 1);
+    return (SDL_RWwrite(dst, &swapped, sizeof(swapped)) == sizeof(swapped)) ? 1 : 0;
 }
 
 size_t
 SDL_WriteBE64(SDL_RWops *dst, Uint64 value)
 {
     const Uint64 swapped = SDL_SwapBE64(value);
-    return SDL_RWwrite(dst, &swapped, sizeof(swapped), 1);
+    return (SDL_RWwrite(dst, &swapped, sizeof(swapped)) == sizeof(swapped)) ? 1 : 0;
 }
 
 /* vi: set ts=4 sw=4 expandtab: */

+ 1 - 1
src/joystick/SDL_gamecontroller.c

@@ -1488,7 +1488,7 @@ int SDL_GameControllerAddMappingsFromRW(SDL_RWops *rw, int freerw)
         return SDL_SetError("Could not allocate space to read DB into memory");
     }
 
-    if (SDL_RWread(rw, buf, db_size, 1) != 1) {
+    if (SDL_RWread(rw, buf, db_size) != db_size) {
         if (freerw) {
             SDL_RWclose(rw);
         }

+ 35 - 26
src/video/SDL_bmp.c

@@ -69,8 +69,9 @@ static SDL_bool readRlePixels(SDL_Surface *surface, SDL_RWops *src, int isRle8)
     if (spot >= start && spot < end) \
     *spot = (x)
 
+    /* !!! FIXME: for all these reads, handle error vs eof? handle -2 if non-blocking? */
     for (;;) {
-        if (!SDL_RWread(src, &ch, 1, 1)) {
+        if (SDL_RWread(src, &ch, 1) <= 0) {
             return SDL_TRUE;
         }
         /*
@@ -79,7 +80,7 @@ static SDL_bool readRlePixels(SDL_Surface *surface, SDL_RWops *src, int isRle8)
         */
         if (ch) {
             Uint8 pixel;
-            if (!SDL_RWread(src, &pixel, 1, 1)) {
+            if (SDL_RWread(src, &pixel, 1) <= 0) {
                 return SDL_TRUE;
             }
             if (isRle8) { /* 256-color bitmap, compressed */
@@ -106,7 +107,7 @@ static SDL_bool readRlePixels(SDL_Surface *surface, SDL_RWops *src, int isRle8)
             | a cursor move, or some absolute data.
             | zero tag may be absolute mode or an escape
             */
-            if (!SDL_RWread(src, &ch, 1, 1)) {
+            if (SDL_RWread(src, &ch, 1) <= 0) {
                 return SDL_TRUE;
             }
             switch (ch) {
@@ -117,11 +118,11 @@ static SDL_bool readRlePixels(SDL_Surface *surface, SDL_RWops *src, int isRle8)
             case 1:               /* end of bitmap */
                 return SDL_FALSE; /* success! */
             case 2:               /* delta */
-                if (!SDL_RWread(src, &ch, 1, 1)) {
+                if (SDL_RWread(src, &ch, 1) <= 0) {
                     return SDL_TRUE;
                 }
                 ofs += ch;
-                if (!SDL_RWread(src, &ch, 1, 1)) {
+                if (SDL_RWread(src, &ch, 1) <= 0) {
                     return SDL_TRUE;
                 }
                 bits -= (ch * pitch);
@@ -131,7 +132,7 @@ static SDL_bool readRlePixels(SDL_Surface *surface, SDL_RWops *src, int isRle8)
                     needsPad = (ch & 1);
                     do {
                         Uint8 pixel;
-                        if (!SDL_RWread(src, &pixel, 1, 1)) {
+                        if (SDL_RWread(src, &pixel, 1) <= 0) {
                             return SDL_TRUE;
                         }
                         COPY_PIXEL(pixel);
@@ -140,7 +141,7 @@ static SDL_bool readRlePixels(SDL_Surface *surface, SDL_RWops *src, int isRle8)
                     needsPad = (((ch + 1) >> 1) & 1); /* (ch+1)>>1: bytes size */
                     for (;;) {
                         Uint8 pixel;
-                        if (!SDL_RWread(src, &pixel, 1, 1)) {
+                        if (SDL_RWread(src, &pixel, 1) <= 0) {
                             return SDL_TRUE;
                         }
                         COPY_PIXEL(pixel >> 4);
@@ -154,7 +155,7 @@ static SDL_bool readRlePixels(SDL_Surface *surface, SDL_RWops *src, int isRle8)
                     }
                 }
                 /* pad at even boundary */
-                if (needsPad && !SDL_RWread(src, &ch, 1, 1)) {
+                if (needsPad && (SDL_RWread(src, &ch, 1) <= 0)) {
                     return SDL_TRUE;
                 }
                 break;
@@ -249,7 +250,7 @@ SDL_LoadBMP_RW(SDL_RWops *src, int freesrc)
         goto done;
     }
     SDL_ClearError();
-    if (SDL_RWread(src, magic, 1, 2) != 2) {
+    if (SDL_RWread(src, magic, 2) != 2) {
         SDL_Error(SDL_EFREAD);
         was_error = SDL_TRUE;
         goto done;
@@ -466,17 +467,19 @@ SDL_LoadBMP_RW(SDL_RWops *src, int freesrc)
 
         if (biSize == 12) {
             for (i = 0; i < (int)biClrUsed; ++i) {
-                SDL_RWread(src, &palette->colors[i].b, 1, 1);
-                SDL_RWread(src, &palette->colors[i].g, 1, 1);
-                SDL_RWread(src, &palette->colors[i].r, 1, 1);
+                /* !!! FIXME: this should check for i/o errors! */
+                SDL_RWread(src, &palette->colors[i].b, 1);
+                SDL_RWread(src, &palette->colors[i].g, 1);
+                SDL_RWread(src, &palette->colors[i].r, 1);
                 palette->colors[i].a = SDL_ALPHA_OPAQUE;
             }
         } else {
             for (i = 0; i < (int)biClrUsed; ++i) {
-                SDL_RWread(src, &palette->colors[i].b, 1, 1);
-                SDL_RWread(src, &palette->colors[i].g, 1, 1);
-                SDL_RWread(src, &palette->colors[i].r, 1, 1);
-                SDL_RWread(src, &palette->colors[i].a, 1, 1);
+                /* !!! FIXME: this should check for i/o errors! */
+                SDL_RWread(src, &palette->colors[i].b, 1);
+                SDL_RWread(src, &palette->colors[i].g, 1);
+                SDL_RWread(src, &palette->colors[i].r, 1);
+                SDL_RWread(src, &palette->colors[i].a, 1);
 
                 /* According to Microsoft documentation, the fourth element
                    is reserved and must be zero, so we shouldn't treat it as
@@ -535,7 +538,7 @@ SDL_LoadBMP_RW(SDL_RWops *src, int freesrc)
             int shift = (8 - ExpandBMP);
             for (i = 0; i < surface->w; ++i) {
                 if (i % (8 / ExpandBMP) == 0) {
-                    if (!SDL_RWread(src, &pixel, 1, 1)) {
+                    if (SDL_RWread(src, &pixel, 1) != 1) {
                         SDL_Error(SDL_EFREAD);
                         was_error = SDL_TRUE;
                         goto done;
@@ -552,7 +555,7 @@ SDL_LoadBMP_RW(SDL_RWops *src, int freesrc)
         } break;
 
         default:
-            if (SDL_RWread(src, bits, 1, surface->pitch) != surface->pitch) {
+            if (SDL_RWread(src, bits, surface->pitch) != surface->pitch) {
                 SDL_Error(SDL_EFREAD);
                 was_error = SDL_TRUE;
                 goto done;
@@ -596,7 +599,7 @@ SDL_LoadBMP_RW(SDL_RWops *src, int freesrc)
         if (pad) {
             Uint8 padbyte;
             for (i = 0; i < pad; ++i) {
-                SDL_RWread(src, &padbyte, 1, 1);
+                SDL_RWread(src, &padbyte, 1);
             }
         }
         if (topDown) {
@@ -624,6 +627,12 @@ done:
 
 int SDL_SaveBMP_RW(SDL_Surface *surface, SDL_RWops *dst, int freedst)
 {
+    /* !!! FIXME: this calls SDL_ClearError() and then checks if an error happened during this function to
+       !!! FIXME: decide if there was a problem, but there's risk of innocent things setting an error
+       !!! FIXME: string for innocent unrelated reasons, and also, an app supplying its own RWops
+       !!! FIXME: implementation may not set the error string on failure. We should check for i/o
+       !!! FIXME: failures as we go, and return early if one occurs. */
+
     Sint64 fp_offset;
     int i, pad;
     SDL_Surface *intermediate_surface;
@@ -731,7 +740,7 @@ int SDL_SaveBMP_RW(SDL_Surface *surface, SDL_RWops *dst, int freedst)
         /* Write the BMP file header values */
         fp_offset = SDL_RWtell(dst);
         SDL_ClearError();
-        SDL_RWwrite(dst, magic, 2, 1);
+        SDL_RWwrite(dst, magic, 2);
         SDL_WriteLE32(dst, bfSize);
         SDL_WriteLE16(dst, bfReserved1);
         SDL_WriteLE16(dst, bfReserved2);
@@ -805,10 +814,10 @@ int SDL_SaveBMP_RW(SDL_Surface *surface, SDL_RWops *dst, int freedst)
             colors = intermediate_surface->format->palette->colors;
             ncolors = intermediate_surface->format->palette->ncolors;
             for (i = 0; i < ncolors; ++i) {
-                SDL_RWwrite(dst, &colors[i].b, 1, 1);
-                SDL_RWwrite(dst, &colors[i].g, 1, 1);
-                SDL_RWwrite(dst, &colors[i].r, 1, 1);
-                SDL_RWwrite(dst, &colors[i].a, 1, 1);
+                SDL_RWwrite(dst, &colors[i].b, 1);
+                SDL_RWwrite(dst, &colors[i].g, 1);
+                SDL_RWwrite(dst, &colors[i].r, 1);
+                SDL_RWwrite(dst, &colors[i].a, 1);
             }
         }
 
@@ -827,14 +836,14 @@ int SDL_SaveBMP_RW(SDL_Surface *surface, SDL_RWops *dst, int freedst)
         pad = ((bw % 4) ? (4 - (bw % 4)) : 0);
         while (bits > (Uint8 *)intermediate_surface->pixels) {
             bits -= intermediate_surface->pitch;
-            if (SDL_RWwrite(dst, bits, 1, bw) != bw) {
+            if (SDL_RWwrite(dst, bits, bw) != bw) {
                 SDL_Error(SDL_EFWRITE);
                 break;
             }
             if (pad) {
                 const Uint8 padbyte = 0;
                 for (i = 0; i < pad; ++i) {
-                    SDL_RWwrite(dst, &padbyte, 1, 1);
+                    SDL_RWwrite(dst, &padbyte, 1);
                 }
             }
         }

+ 9 - 9
test/testautomation_rwops.c

@@ -99,7 +99,7 @@ void _testGenericRWopsValidations(SDL_RWops *rw, int write)
 {
     char buf[sizeof(RWopsHelloWorldTestString)];
     Sint64 i;
-    size_t s;
+    Sint64 s;
     int seekPos = SDLTest_RandomIntegerInRange(4, 8);
 
     /* Clear buffer */
@@ -111,12 +111,12 @@ void _testGenericRWopsValidations(SDL_RWops *rw, int write)
     SDLTest_AssertCheck(i == (Sint64)0, "Verify seek to 0 with SDL_RWseek (RW_SEEK_SET), expected 0, got %" SDL_PRIs64, i);
 
     /* Test write. */
-    s = SDL_RWwrite(rw, RWopsHelloWorldTestString, sizeof(RWopsHelloWorldTestString) - 1, 1);
+    s = SDL_RWwrite(rw, RWopsHelloWorldTestString, sizeof(RWopsHelloWorldTestString) - 1);
     SDLTest_AssertPass("Call to SDL_RWwrite succeeded");
     if (write) {
-        SDLTest_AssertCheck(s == (size_t)1, "Verify result of writing one byte with SDL_RWwrite, expected 1, got %i", (int)s);
+        SDLTest_AssertCheck(s == sizeof(RWopsHelloWorldTestString) - 1, "Verify result of writing one byte with SDL_RWwrite, expected 1, got %i", (int)s);
     } else {
-        SDLTest_AssertCheck(s == (size_t)0, "Verify result of writing with SDL_RWwrite, expected: 0, got %i", (int)s);
+        SDLTest_AssertCheck(s == 0, "Verify result of writing with SDL_RWwrite, expected: 0, got %i", (int)s);
     }
 
     /* Test seek to random position */
@@ -130,7 +130,7 @@ void _testGenericRWopsValidations(SDL_RWops *rw, int write)
     SDLTest_AssertCheck(i == (Sint64)0, "Verify seek to 0 with SDL_RWseek (RW_SEEK_SET), expected 0, got %" SDL_PRIs64, i);
 
     /* Test read */
-    s = SDL_RWread(rw, buf, 1, sizeof(RWopsHelloWorldTestString) - 1);
+    s = SDL_RWread(rw, buf, sizeof(RWopsHelloWorldTestString) - 1);
     SDLTest_AssertPass("Call to SDL_RWread succeeded");
     SDLTest_AssertCheck(
         s == (size_t)(sizeof(RWopsHelloWorldTestString) - 1),
@@ -440,8 +440,8 @@ int rwops_testCompareRWFromMemWithRWFromFile(void)
         /* Read/seek from memory */
         rwops_mem = SDL_RWFromMem((void *)RWopsAlphabetString, slen);
         SDLTest_AssertPass("Call to SDL_RWFromMem()");
-        rv_mem = SDL_RWread(rwops_mem, buffer_mem, size, 6);
-        SDLTest_AssertPass("Call to SDL_RWread(mem, size=%d)", size);
+        rv_mem = SDL_RWread(rwops_mem, buffer_mem, size * 6);
+        SDLTest_AssertPass("Call to SDL_RWread(mem, size=%d)", size * 6);
         sv_mem = SDL_RWseek(rwops_mem, 0, SEEK_END);
         SDLTest_AssertPass("Call to SDL_RWseek(mem,SEEK_END)");
         result = SDL_RWclose(rwops_mem);
@@ -451,8 +451,8 @@ int rwops_testCompareRWFromMemWithRWFromFile(void)
         /* Read/see from file */
         rwops_file = SDL_RWFromFile(RWopsAlphabetFilename, "r");
         SDLTest_AssertPass("Call to SDL_RWFromFile()");
-        rv_file = SDL_RWread(rwops_file, buffer_file, size, 6);
-        SDLTest_AssertPass("Call to SDL_RWread(file, size=%d)", size);
+        rv_file = SDL_RWread(rwops_file, buffer_file, size * 6);
+        SDLTest_AssertPass("Call to SDL_RWread(file, size=%d)", size * 6);
         sv_file = SDL_RWseek(rwops_file, 0, SEEK_END);
         SDLTest_AssertPass("Call to SDL_RWseek(file,SEEK_END)");
         result = SDL_RWclose(rwops_file);

+ 3 - 3
test/testime.c

@@ -145,7 +145,7 @@ static int unifont_init(const char *fontname)
         Uint8 glyphWidth;
         Uint32 codepoint;
 
-        bytesRead = SDL_RWread(hexFile, hexBuffer, 1, 9);
+        bytesRead = SDL_RWread(hexFile, hexBuffer, 9);
         if (numGlyphs > 0 && bytesRead == 0) {
             break; /* EOF */
         }
@@ -181,7 +181,7 @@ static int unifont_init(const char *fontname)
         if (codepointHexSize < 8) {
             SDL_memmove(hexBuffer, hexBuffer + codepointHexSize + 1, bytesOverread);
         }
-        bytesRead = SDL_RWread(hexFile, hexBuffer + bytesOverread, 1, 33 - bytesOverread);
+        bytesRead = SDL_RWread(hexFile, hexBuffer + bytesOverread, 33 - bytesOverread);
         if (bytesRead < (33 - bytesOverread)) {
             SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "unifont: Unexpected end of hex file.\n");
             return -1;
@@ -190,7 +190,7 @@ static int unifont_init(const char *fontname)
             glyphWidth = 8;
         } else {
             glyphWidth = 16;
-            bytesRead = SDL_RWread(hexFile, hexBuffer + 33, 1, 32);
+            bytesRead = SDL_RWread(hexFile, hexBuffer + 33, 32);
             if (bytesRead < 32) {
                 SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "unifont: Unexpected end of hex file.\n");
                 return -1;

+ 1 - 1
test/testoverlay2.c

@@ -420,7 +420,7 @@ int main(int argc, char **argv)
         quit(2);
     }
 
-    SDL_RWread(handle, RawMooseData, MOOSEFRAME_SIZE, MOOSEFRAMES_COUNT);
+    SDL_RWread(handle, RawMooseData, MOOSEFRAME_SIZE * MOOSEFRAMES_COUNT);
 
     SDL_RWclose(handle);
 

+ 1 - 1
test/testresample.c

@@ -100,7 +100,7 @@ int main(int argc, char **argv)
     SDL_WriteLE16(io, bitsize);                                /* significant bits per sample */
     SDL_WriteLE32(io, 0x61746164);                             /* data */
     SDL_WriteLE32(io, cvt.len_cvt);                            /* size */
-    SDL_RWwrite(io, cvt.buf, cvt.len_cvt, 1);
+    SDL_RWwrite(io, cvt.buf, cvt.len_cvt);
 
     if (SDL_RWclose(io) == -1) {
         SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "fclose('%s') failed: %s\n", argv[2], SDL_GetError());

+ 1 - 1
test/teststreaming.c

@@ -150,7 +150,7 @@ int main(int argc, char **argv)
         SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Can't find the file moose.dat !\n");
         quit(2);
     }
-    SDL_RWread(handle, MooseFrames, MOOSEFRAME_SIZE, MOOSEFRAMES_COUNT);
+    SDL_RWread(handle, MooseFrames, MOOSEFRAME_SIZE * MOOSEFRAMES_COUNT);
     SDL_RWclose(handle);
 
     /* Create the window and renderer */