Skip to content

Commit 8b9a267

Browse files
committed
Fixed error handling in dip::ImageReadJPEG(), dip::ImageWriteJPEG(), etc. Fixes #80.
1 parent d7d7155 commit 8b9a267

File tree

3 files changed

+37
-26
lines changed

3 files changed

+37
-26
lines changed

Diff for: CMakeLists.txt

+3
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang") # also matchs "AppleClang"
4343
# Compiler flags for Clang C++
4444
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wconversion -Wsign-conversion -pedantic -Wno-c++17-extensions")
4545
#set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -march=native") # This is optimal for local usage.
46+
set(CMAKE_C_FLAGS_SANITIZE "${CMAKE_C_FLAGS_DEBUG} -fsanitize=address")
4647
set(CMAKE_CXX_FLAGS_SANITIZE "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address")
4748
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
4849
# Compiler flags for GNU C++
@@ -53,7 +54,9 @@ elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
5354
# "DIP_EXPORT" in forward class declaration sometimes causes a warning in GCC 6.0 and 7.0.
5455
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-attributes")
5556
#set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -march=native") # This is optimal for local usage; to see which flags are enabled: gcc -march=native -Q --help=target
57+
set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -Og") # Does some optimization that doesn't impact debugging.
5658
set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -Og") # Does some optimization that doesn't impact debugging.
59+
set(CMAKE_C_FLAGS_SANITIZE "${CMAKE_C_FLAGS_DEBUG} -fsanitize=address")
5760
set(CMAKE_CXX_FLAGS_SANITIZE "${CMAKE_CXX_FLAGS_DEBUG} -fsanitize=address")
5861
elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Intel")
5962
# Compiler flags for Intel C++

Diff for: changelogs/diplib_3.1.0.md

+2
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,8 @@ title: "Changes DIPlib 3.1.0"
102102

103103
- libics had a typo that caused out-of-bounds read (#81).
104104

105+
- Fixed error handling in `dip::ImageReadJPEG()` and `dip::ImageWriteJPEG()`, which previously would crash when libjpeg produced an error (#80).
106+
105107

106108

107109

Diff for: src/file_io/jpeg.cpp

+32-26
Original file line numberDiff line numberDiff line change
@@ -24,31 +24,40 @@
2424
#include "diplib/file_io.h"
2525

2626
#include "jpeglib.h"
27-
#include <setjmp.h>
28-
29-
namespace dip {
30-
31-
namespace {
27+
#include <csetjmp>
3228

3329
// JPEG error handling stuff - modified from example.c in libjpeg source
30+
extern "C" {
31+
static void my_error_exit( j_common_ptr cinfo );
32+
static void my_output_message( j_common_ptr );
33+
}
34+
3435
struct my_error_mgr {
35-
struct jpeg_error_mgr pub; // "public" fields
36-
jmp_buf setjmp_buffer; // for return to caller
36+
struct jpeg_error_mgr pub; // "public" fields
37+
std::jmp_buf setjmp_buffer; // for return to caller
3738
};
3839
using my_error_ptr = struct my_error_mgr*;
3940

40-
void my_error_exit( j_common_ptr cinfo ) {
41+
static void my_error_exit( j_common_ptr cinfo ) {
4142
// cinfo->err really points to a my_error_mgr struct, so coerce pointer
42-
my_error_ptr myerr = reinterpret_cast<my_error_ptr>(cinfo->err);
43+
my_error_ptr myerr = reinterpret_cast<my_error_ptr>( cinfo->err );
4344
// Return control to the setjmp point
4445
longjmp( myerr->setjmp_buffer, 1 );
4546
}
4647

47-
void my_output_message( j_common_ptr ) {} // Don't do anything with messages!
48+
static void my_output_message( j_common_ptr ) {} // Don't do anything with messages!
49+
50+
#define DIP__DECLARE_JPEG_EXIT( message ) \
51+
std::jmp_buf setjmp_buffer; if( setjmp( setjmp_buffer )) { DIP_THROW_RUNTIME( message ); }
52+
53+
54+
namespace dip {
55+
56+
namespace {
4857

4958
class JpegInput {
5059
public:
51-
JpegInput( String filename ) : filename_( std::move( filename )) {
60+
JpegInput( String filename, std::jmp_buf const& setjmp_buffer ) : filename_( std::move( filename )) {
5261
infile_ = std::fopen( filename_.c_str(), "rb" );
5362
if( infile_ == nullptr ) {
5463
if( !FileHasExtension( filename_ )) {
@@ -66,10 +75,7 @@ class JpegInput {
6675
cinfo_.err = jpeg_std_error( &jerr_.pub );
6776
jerr_.pub.error_exit = my_error_exit;
6877
jerr_.pub.output_message = my_output_message;
69-
if( setjmp( jerr_.setjmp_buffer )) {
70-
// If we get here, the JPEG code has signaled an error.
71-
DIP_THROW_RUNTIME( "Error reading JPEG file." );
72-
}
78+
std::memcpy( jerr_.setjmp_buffer, setjmp_buffer, sizeof( setjmp_buffer ));
7379
jpeg_create_decompress( &cinfo_ );
7480
initialized_ = true;
7581
jpeg_stdio_src( &cinfo_, infile_ );
@@ -102,7 +108,7 @@ class JpegInput {
102108

103109
class JpegOutput {
104110
public:
105-
explicit JpegOutput( String const& filename ) {
111+
explicit JpegOutput( String const& filename, std::jmp_buf const& setjmp_buffer ) {
106112
// Open the file for writing
107113
if( FileHasExtension( filename )) {
108114
outfile_ = std::fopen(filename.c_str(), "wb");
@@ -115,10 +121,7 @@ class JpegOutput {
115121
cinfo_.err = jpeg_std_error( &jerr_.pub );
116122
jerr_.pub.error_exit = my_error_exit;
117123
jerr_.pub.output_message = my_output_message;
118-
if( setjmp( jerr_.setjmp_buffer )) {
119-
// If we get here, the JPEG code has signaled an error.
120-
DIP_THROW_RUNTIME( "Error writing JPEG file." );
121-
}
124+
std::memcpy( jerr_.setjmp_buffer, setjmp_buffer, sizeof( setjmp_buffer ));
122125
jpeg_create_compress( &cinfo_ );
123126
initialized_ = true;
124127
jpeg_stdio_dest( &cinfo_, outfile_ );
@@ -178,7 +181,8 @@ FileInformation ImageReadJPEG(
178181
String const& filename
179182
) {
180183
// Open the file
181-
JpegInput jpeg( filename );
184+
DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
185+
JpegInput jpeg( filename, setjmp_buffer );
182186

183187
// Get info
184188
FileInformation info = GetJPEGInfo( jpeg );
@@ -223,14 +227,16 @@ FileInformation ImageReadJPEG(
223227
}
224228

225229
FileInformation ImageReadJPEGInfo( String const& filename ) {
226-
JpegInput jpeg( filename );
230+
DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
231+
JpegInput jpeg( filename, setjmp_buffer );
227232
FileInformation info = GetJPEGInfo( jpeg );
228233
return info;
229234
}
230235

231236
bool ImageIsJPEG( String const& filename ) {
232237
try {
233-
JpegInput jpeg( filename );
238+
DIP__DECLARE_JPEG_EXIT( "Error reading JPEG file" );
239+
JpegInput jpeg( filename, setjmp_buffer );
234240
} catch( ... ) {
235241
return false;
236242
}
@@ -244,10 +250,10 @@ void ImageWriteJPEG(
244250
) {
245251
DIP_THROW_IF( !image.IsForged(), E::IMAGE_NOT_FORGED );
246252
DIP_THROW_IF( image.Dimensionality() != 2, E::DIMENSIONALITY_NOT_SUPPORTED );
247-
jpegLevel = clamp< dip::uint >( jpegLevel, 1, 100 );
248253

249254
// Open the file
250-
JpegOutput jpeg( filename );
255+
DIP__DECLARE_JPEG_EXIT( "Error writing JPEG file" );
256+
JpegOutput jpeg( filename, setjmp_buffer );
251257

252258
// Set image properties
253259
int nchan = static_cast< int >( image.TensorElements() );
@@ -256,7 +262,7 @@ void ImageWriteJPEG(
256262
jpeg.cinfo().input_components = nchan;
257263
jpeg.cinfo().in_color_space = nchan > 1 ? JCS_RGB : JCS_GRAYSCALE;
258264
jpeg_set_defaults( jpeg.cinfoptr() );
259-
jpeg_set_quality( jpeg.cinfoptr(), static_cast< int >( jpegLevel ), FALSE );
265+
jpeg_set_quality( jpeg.cinfoptr(), static_cast< int >( clamp< dip::uint >( jpegLevel, 1, 100 )), FALSE );
260266
jpeg.cinfo().density_unit = 2; // dots per cm
261267
jpeg.cinfo().X_density = static_cast< UINT16 >( 0.01 / image.PixelSize( 0 ).RemovePrefix().magnitude ); // let's assume it's meter
262268
jpeg.cinfo().Y_density = static_cast< UINT16 >( 0.01 / image.PixelSize( 1 ).RemovePrefix().magnitude );

0 commit comments

Comments
 (0)