Tidy up opj_ callback functions in fx_codec_jpx_obj.cpp

This is code cleanup rather than bug fixing.

The motivation for this was to fix the casts at line 97 of the original file.  These are wrong; you cannot correct via casting a function signature mismatch when passing a function as an argument.  In theory, there's no reason to believe that the compiler will pass args in the same manner for a function of type (void*, size_t, void*) as for a function of type (void*, size_t, some_struct*).  The cast will suppress the compile error, but you can't be assured the call will work as intended.  In practice, it does, since the last architecture where a void* had a different representation than a struct* went extinct in the late 80s.

In the functions themselves, note that we currently bail out if srcData->offset >= srcData->src_size, so the expression
   bufferLength = (OPJ_SIZE_T)(srcData->src_size - srcData->offset)

will always be > 0.  Hence the check
   if(bufferLength <= 0)
is pointless, esp. since bufferLength is a signed type and < 0 makes no sense.

The opj_seek_from_memory() has a bool return value, so returning -1 on error doesn't seem reasonable.  Change this to TRUE/FALSE, and return false on seek past end.

If we're truly passing readonly data, then perhaps it makes sense to make the write() function always return -1. I didn't do this.

Lastly, I capitalize "DecodeData" so that it looks like a struct, and change its members to be size_t's to avoid casting back and forth.

R=jun_fang@foxitsoftware.com

Review URL: https://codereview.chromium.org/507273003

1 files changed, 40 insertions, 50 deletions

diff --git a/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp b/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp
index 5ae75557cd..595d4df652 100644
--- a/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp
+++ b/core/src/fxcodec/codec/fx_codec_jpx_opj.cpp
@@ -20,69 +20,59 @@ static void fx_info_callback(const char *msg, void *client_data)
 {
     (void)client_data;
 }
-typedef struct {
-    const unsigned char* src_data;
-    int					 src_size;
-    int					 offset;
-} decodeData;
-static OPJ_SIZE_T opj_read_from_memory (void * p_buffer, OPJ_SIZE_T p_nb_bytes,  decodeData* srcData)
+struct DecodeData {
+    unsigned char* src_data;
+    OPJ_SIZE_T     src_size;
+    OPJ_SIZE_T     offset;
+};
+static OPJ_SIZE_T opj_read_from_memory (void * p_buffer, OPJ_SIZE_T p_nb_bytes,  void* p_user_data)
 {
-    if(srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL || srcData->offset >= srcData->src_size) {
+    DecodeData* srcData = static_cast<DecodeData*>(p_user_data);
+    if (srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL || srcData->offset >= srcData->src_size) {
         return -1;
     }
-    OPJ_SIZE_T readlength = p_nb_bytes;
-    OPJ_SIZE_T bufferLength = (OPJ_SIZE_T)(srcData->src_size - srcData->offset);
-    if(bufferLength <= 0) {
-        return 0;
-    }
-    if(bufferLength <= p_nb_bytes) {
-        readlength = bufferLength;
-    }
-    memcpy(p_buffer, &(srcData->src_data[srcData->offset]), readlength);
-    srcData->offset += (int)readlength;
+    OPJ_SIZE_T bufferLength = srcData->src_size - srcData->offset;
+    OPJ_SIZE_T readlength = p_nb_bytes < bufferLength ? p_nb_bytes : bufferLength;
+    memcpy(p_buffer, &srcData->src_data[srcData->offset], readlength);
+    srcData->offset += readlength;
     return readlength;
 }
-static OPJ_SIZE_T opj_write_from_memory (void * p_buffer, OPJ_SIZE_T p_nb_bytes, decodeData* srcData)
+static OPJ_SIZE_T opj_write_from_memory (void * p_buffer, OPJ_SIZE_T p_nb_bytes, void* p_user_data)
 {
-    if(srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL || srcData->offset >= srcData->src_size) {
+    DecodeData* srcData = static_cast<DecodeData*>(p_user_data);
+    if (srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL || srcData->offset >= srcData->src_size) {
         return -1;
     }
-    OPJ_SIZE_T writeLength = p_nb_bytes;
-    OPJ_SIZE_T bufferLength = (OPJ_SIZE_T)(srcData->src_size - srcData->offset);
-    if(bufferLength <= p_nb_bytes) {
-        writeLength = bufferLength;
-    }
-    memcpy((void*&)(srcData->src_data[srcData->offset]), p_buffer, writeLength);
-    srcData->offset += (int)writeLength;
+    OPJ_SIZE_T bufferLength = srcData->src_size - srcData->offset;
+    OPJ_SIZE_T writeLength = p_nb_bytes < bufferLength ? p_nb_bytes : bufferLength;
+    memcpy(&srcData->src_data[srcData->offset], p_buffer, writeLength);
+    srcData->offset += writeLength;
     return writeLength;
 }
-static OPJ_OFF_T opj_skip_from_memory (OPJ_OFF_T p_nb_bytes, decodeData* srcData)
+static OPJ_OFF_T opj_skip_from_memory (OPJ_OFF_T p_nb_bytes, void* p_user_data)
 {
-    if(srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL || srcData->offset >= srcData->src_size) {
+    DecodeData* srcData = static_cast<DecodeData*>(p_user_data);
+    if (srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL || srcData->offset >= srcData->src_size) {
         return -1;
     }
-    OPJ_OFF_T postion = srcData->offset + p_nb_bytes;
-    if(postion < 0 ) {
-        postion = 0;
-    } else if (postion > srcData->src_size) {
-    }
-    srcData->offset = (int)postion;
-    return p_nb_bytes;
+    OPJ_SIZE_T bufferLength = srcData->src_size - srcData->offset;
+    OPJ_SIZE_T skipLength = p_nb_bytes < bufferLength ? p_nb_bytes : bufferLength;
+    srcData->offset += skipLength;
+    return skipLength;
 }
-static OPJ_BOOL opj_seek_from_memory (OPJ_OFF_T p_nb_bytes, decodeData * srcData)
+static OPJ_BOOL opj_seek_from_memory (OPJ_OFF_T p_nb_bytes, void* p_user_data)
 {
-    if(srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL || srcData->offset >= srcData->src_size) {
-        return -1;
+    DecodeData* srcData = static_cast<DecodeData*>(p_user_data);
+    if (srcData == NULL || srcData->src_size == 0 || srcData->src_data == NULL || srcData->offset >= srcData->src_size) {
+        return OPJ_FALSE;
     }
-    srcData->offset = (int)p_nb_bytes;
-    if(srcData->offset < 0) {
-        srcData->offset = 0;
-    } else if(srcData->offset > srcData->src_size) {
-        srcData->offset = srcData->src_size;
+    if (p_nb_bytes >= srcData->src_size) {
+        return OPJ_FALSE;
     }
+    srcData->offset = p_nb_bytes;
     return OPJ_TRUE;
 }
-opj_stream_t* fx_opj_stream_create_memory_stream (decodeData* data,	OPJ_SIZE_T p_size, 	OPJ_BOOL p_is_read_stream)
+opj_stream_t* fx_opj_stream_create_memory_stream (DecodeData* data,	OPJ_SIZE_T p_size, 	OPJ_BOOL p_is_read_stream)
 {
     opj_stream_t* l_stream = 00;
     if (!data || ! data->src_data || data->src_size <= 0 ) {
@@ -94,10 +84,10 @@ opj_stream_t* fx_opj_stream_create_memory_stream (decodeData* data,	OPJ_SIZE_T p
     }
     opj_stream_set_user_data_v3(l_stream, data, NULL);
     opj_stream_set_user_data_length(l_stream, data->src_size);
-    opj_stream_set_read_function(l_stream, (opj_stream_read_fn) opj_read_from_memory);
-    opj_stream_set_write_function(l_stream, (opj_stream_write_fn) opj_write_from_memory);
-    opj_stream_set_skip_function(l_stream, (opj_stream_skip_fn) opj_skip_from_memory);
-    opj_stream_set_seek_function(l_stream, (opj_stream_seek_fn) opj_seek_from_memory);
+    opj_stream_set_read_function(l_stream, opj_read_from_memory);
+    opj_stream_set_write_function(l_stream, opj_write_from_memory);
+    opj_stream_set_skip_function(l_stream, opj_skip_from_memory);
+    opj_stream_set_seek_function(l_stream, opj_seek_from_memory);
     return l_stream;
 }
 static void sycc_to_rgb(int offset, int upb, int y, int cb, int cr,
@@ -588,10 +578,10 @@ FX_BOOL CJPX_Decoder::Init(const unsigned char* src_data, int src_size)
     image = NULL;
     m_SrcData = src_data;
     m_SrcSize = src_size;
-    decodeData srcData;
+    DecodeData srcData;
     srcData.offset  = 0;
     srcData.src_size = src_size;
-    srcData.src_data = src_data;
+    srcData.src_data = const_cast<unsigned char*>(src_data);
     l_stream = fx_opj_stream_create_memory_stream(&srcData, OPJ_J2K_STREAM_CHUNK_SIZE, 1);
     if (l_stream == NULL) {
         return FALSE;