Two changes here:
1) use client_data structure, rather than assume the cinfo->err structure has extra space after it which holds the longjmp/setjmp buffer. (This is not a fix but a cleanup.)
2) make sure to return an error code in the event the thumbnail decoding fails.
By the way, is this the right place to submit bug reports?
--- src/libraw_cxx.cpp.dist 2018-02-27 22:35:04.000000000 -0800
+++ src/libraw_cxx.cpp 2018-03-18 23:38:39.000000000 -0700
@@ -4171,7 +4171,7 @@
static void jpegErrorExit(j_common_ptr cinfo)
{
- jpegErrorManager *myerr = (jpegErrorManager *)cinfo->err;
+ jpegErrorManager *myerr = (jpegErrorManager *)cinfo->client_data;
longjmp(myerr->setjmp_buffer, 1);
}
#endif
@@ -4238,6 +4238,7 @@
{
jpegErrorManager jerr;
struct jpeg_decompress_struct cinfo;
+ cinfo.client_data = &jerr;
cinfo.err = jpeg_std_error(&jerr.pub);
jerr.pub.error_exit = jpegErrorExit;
if (setjmp(jerr.setjmp_buffer))
@@ -4245,6 +4246,7 @@
err2:
jpeg_destroy_decompress(&cinfo);
T.tcolors = 3;
+ return LIBRAW_DATA_ERROR;
}
jpeg_create_decompress(&cinfo);
jpeg_mem_src(&cinfo, (unsigned char *)T.thumb, T.tlength);
JPEG thumbnails are provided
JPEG thumbnails are provided as-is, libjpeg is used only to get channels count.
So, even if libjpeg fails, this does not mean bad thumbnail, calling app should test it (again)
-- Alex Tutubalin @LibRaw LLC
You still need the return
You still need the return statement, otherwise if jpeg_create_decompress fails, won't it fail again after the longjmp?
Agree, it may fail in bad
Agree, it may fail in bad loop here. To be fixed in beta2
return definitely needed here, also error should be indicated by
T.tformat = LIBRAW_THUMBNAIL_UNKNOWN;
-- Alex Tutubalin @LibRaw LLC
This patch fixes it right way
This patch fixes it right way: https://github.com/LibRaw/LibRaw/commit/3d1dba8dcbf8ea533f44cc06354846ed...
-- Alex Tutubalin @LibRaw LLC
I agree this is a much better
I agree this is a much better fix and will apply it to my copy. Thanks!
And 1st patch:
And 1st patch:
cinfo.err is pointer, it does not need to have 'space' for longjump, it just points to already allocated structure.
-- Alex Tutubalin @LibRaw LLC
With respect to cinfo.err,
With respect to cinfo.err, the current code does work.
However, if you look at the definition of cinfo.err in libjpeg, it is assumed to be a pointer to a structure that contains function pointers for the error handler, exit handler, etc. The cerr pointer you pass in has that structure as its first part, but then also has a jmp_buf structure following this structure as the second part.
Somehow to my mind it is cleaner to have cinfo.err point to a "pure" function pointers block, and use cinfo.client_data to point to the jmp_buf structure. But to each his own.
And, yes, this points to two
And, yes, this points to two-part structure: pointers table + additional jump buffer.
I do not see anything bad here.
-- Alex Tutubalin @LibRaw LLC