api: Add OIIO_NODISCARD to ImageInput create/open/read methods (#5111)

## Summary

Adds `OIIO_NODISCARD` to `ImageInput::create()`, `open()`,
`read_image()`, and `read_scanlines()` methods. Callers ignoring return
values from these methods now get a compiler warning.

## Why this matters

As noted in #4781, it's easy to accidentally ignore the bool return from
`read_image()` or `open()`. These methods return false on failure, and
silently proceeding with bad data leads to hard-to-debug issues.
`create()` returns a unique_ptr that should never be discarded.

## Changes

- `src/include/OpenImageIO/imageio.h`: Added `OIIO_NODISCARD` to 14
method declarations in the `ImageInput` class, covering `create()` (2
overloads), `open()` (4 overloads), `read_image()` (4 overloads), and
`read_scanlines()` (4 overloads).

Scoped to `ImageInput` only per @lgritz's guidance to start with one
class and iterate.

## Testing

- Header compiles (syntax validated)
- No behavioral changes - annotation only

Partial fix for #4781

Assisted-by: Claude Code / Claude Opus 4.6 (1M context) 

Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Signed-off-by: Larry Gritz <lg@larrygritz.com>
Co-authored-by: Larry Gritz <lg@larrygritz.com>

Author: mvanhorn

src/include/OpenImageIO/imageio.h

@@ -1180,10 +1180,11 @@ class OIIO_API ImageInput {
     ///
     /// @returns
     ///         `true` if the file was found and opened successfully.
-    virtual bool open (const std::string& name, ImageSpec &newspec) = 0;
+    OIIO_NODISCARD_ERROR virtual bool open (const std::string& name,
+                                            ImageSpec &newspec) = 0;
 
     /// Open the ImageInput using a UTF-16 encoded wstring filename.
-    bool open (const std::wstring& name, ImageSpec &newspec) {
+    OIIO_NODISCARD_ERROR bool open (const std::wstring& name, ImageSpec &newspec) {
         return open(Strutil::utf16_to_utf8(name), newspec);
     }
 
@@ -1207,13 +1208,14 @@ class OIIO_API ImageInput {
     ///
     /// @returns
     ///         `true` if the file was found and opened successfully.
-    virtual bool open (const std::string& name, ImageSpec &newspec,
-                       const ImageSpec& config OIIO_MAYBE_UNUSED) {
+    OIIO_NODISCARD_ERROR virtual bool open (const std::string& name,
+                                            ImageSpec &newspec,
+                                            const ImageSpec& config OIIO_MAYBE_UNUSED) {
         return open(name,newspec);
     }
     /// Open the ImageInput using a UTF-16 encoded wstring filename.
-    bool open (const std::wstring& name, ImageSpec &newspec,
-               const ImageSpec& config OIIO_MAYBE_UNUSED) {
+    OIIO_NODISCARD_ERROR bool open (const std::wstring& name, ImageSpec &newspec,
+                                    const ImageSpec& config OIIO_MAYBE_UNUSED) {
         return open(name,newspec);
     }
 
@@ -1411,15 +1413,17 @@ class OIIO_API ImageInput {
     ///                     y, and z).
     /// @returns            `true` upon success, or `false` upon failure.
     ///
-    virtual bool read_image(int subimage, int miplevel, int chbegin, int chend,
-                            TypeDesc format, const image_span<std::byte>& data);
+    OIIO_NODISCARD_ERROR virtual bool
+    read_image(int subimage, int miplevel, int chbegin, int chend,
+               TypeDesc format, const image_span<std::byte>& data);
 
     /// A version of `read_image()` taking an `image_span<T>`, where the type
     /// of the underlying data is `T`.  This is a convenience wrapper around
     /// the `read_image()` that takes an `image_span<std::byte>`.
     template<typename T>
-    bool read_image(int subimage, int miplevel, int chbegin, int chend,
-                    const image_span<T>& data)
+    OIIO_NODISCARD_ERROR bool read_image(int subimage, int miplevel,
+                                         int chbegin, int chend,
+                                         const image_span<T>& data)
     {
         static_assert(!std::is_const_v<T>,
                       "read_image() does not accept image_span<const T>");
@@ -1432,8 +1436,8 @@ class OIIO_API ImageInput {
     /// contiguous strides in all dimensions. This is a convenience wrapper
     /// around the `read_image()` that takes an `image_span<T>`.
     template<typename T>
-    bool read_image(int subimage, int miplevel, int chbegin, int chend,
-                    span<T> data)
+    OIIO_NODISCARD_ERROR bool read_image(int subimage, int miplevel,
+                                         int chbegin, int chend, span<T> data)
     {
         static_assert(!std::is_const_v<T>,
                       "read_image() does not accept span<const T>");
@@ -1480,17 +1484,18 @@ class OIIO_API ImageInput {
     /// Added in OIIO 3.1, this is the "safe" preferred alternative to
     /// the version of read_scanlines that takes raw pointers.
     ///
-    virtual bool read_scanlines(int subimage, int miplevel, int ybegin,
-                                int yend, int chbegin, int chend,
-                                TypeDesc format,
-                                const image_span<std::byte>& data);
+    OIIO_NODISCARD_ERROR virtual bool
+    read_scanlines(int subimage, int miplevel, int ybegin, int yend,
+                   int chbegin, int chend, TypeDesc format,
+                   const image_span<std::byte>& data);
 
     /// A version of `read_scanlines()` taking an `image_span<T>`, where the
     /// type of the underlying data is `T`.  This is a convenience wrapper
     /// around the `read_scanlines()` that takes an `image_span<std::byte>`.
     template<typename T>
-    bool read_scanlines(int subimage, int miplevel, int ybegin, int yend,
-                        int chbegin, int chend, const image_span<T>& data)
+    OIIO_NODISCARD_ERROR bool
+    read_scanlines(int subimage, int miplevel, int ybegin, int yend,
+                   int chbegin, int chend, const image_span<T>& data)
     {
         static_assert(!std::is_const_v<T>,
                       "read_scanlines() does not accept span<const T>");
@@ -1504,8 +1509,9 @@ class OIIO_API ImageInput {
     /// contiguous strides in all dimensions. This is a convenience wrapper
     /// around the `read_scanlines()` that takes an `image_span<T>`.
     template<typename T>
-    bool read_scanlines(int subimage, int miplevel, int ybegin, int yend,
-                        int chbegin, int chend, span<T> data)
+    OIIO_NODISCARD_ERROR bool read_scanlines(int subimage, int miplevel,
+                                             int ybegin, int yend, int chbegin,
+                                             int chend, span<T> data)
     {
         static_assert(!std::is_const_v<T>,
                       "read_scanlines() does not accept span<const T>");
@@ -1773,12 +1779,12 @@ class OIIO_API ImageInput {
     ///
     /// @note This call was changed for OpenImageIO 2.0 to include the
     ///     explicit subimage and miplevel parameters.
-    virtual bool read_scanlines (int subimage, int miplevel,
-                                 int ybegin, int yend, int z,
-                                 int chbegin, int chend,
-                                 TypeDesc format, void *data,
-                                 stride_t xstride=AutoStride,
-                                 stride_t ystride=AutoStride);
+    OIIO_NODISCARD_ERROR virtual bool read_scanlines (int subimage, int miplevel,
+                                                int ybegin, int yend, int z,
+                                                int chbegin, int chend,
+                                                TypeDesc format, void *data,
+                                                stride_t xstride=AutoStride,
+                                                stride_t ystride=AutoStride);
 
     /// Read the tile whose upper-left origin is (x,y,z) into `data[]`,
     /// converting if necessary from the native data format of the file into
@@ -1904,14 +1910,14 @@ class OIIO_API ImageInput {
     /// @param  progress_callback/progress_callback_data
     ///                     Optional progress callback.
     /// @returns            `true` upon success, or `false` upon failure.
-    virtual bool read_image (int subimage, int miplevel,
-                             int chbegin, int chend,
-                             TypeDesc format, void *data,
-                             stride_t xstride=AutoStride,
-                             stride_t ystride=AutoStride,
-                             stride_t zstride=AutoStride,
-                             ProgressCallback progress_callback=NULL,
-                             void *progress_callback_data=NULL);
+    OIIO_NODISCARD_ERROR virtual bool read_image (int subimage, int miplevel,
+                                            int chbegin, int chend,
+                                            TypeDesc format, void *data,
+                                            stride_t xstride=AutoStride,
+                                            stride_t ystride=AutoStride,
+                                            stride_t zstride=AutoStride,
+                                            ProgressCallback progress_callback=NULL,
+                                            void *progress_callback_data=NULL);
 
     /// @}
 

src/include/OpenImageIO/platform.h

@@ -466,6 +466,28 @@
 #    define OIIO_NODISCARD
 #endif
 
+// OIIO_NODISCARD_ERROR is for functions returning error status (bool) where
+// ignoring the return is a bad practice but not always catastrophic.
+//
+// OIIO_NODISCARD_ERROR_ENABLE, if nonzero, enables OIIO_NODISCARD_ERROR to
+// take effect. The default is to disable it for OIIO < 3.3, and enable it for
+// OIIO >= 3.3. But `-DOIIO_NODISCARD_ERROR_ENABLE=...` can be used to
+// override the default, for example to flag discarded errors in older
+// versions of OIIO, or to disable the warnings in future versions of OIIO.
+#ifndef OIIO_NODISCARD_ERROR_ENABLE
+#    if OIIO_VERSION_LESS(3, 3, 0)
+#        define OIIO_NODISCARD_ERROR_ENABLE 0 /* disable for now */
+#    else
+#        define OIIO_NODISCARD_ERROR_ENABLE 1 /* enable for OIIO >= 3.3 */
+#    endif
+#endif
+
+#if OIIO_NODISCARD_ERROR_ENABLE
+#    define OIIO_NODISCARD_ERROR OIIO_NODISCARD
+#else
+#    define OIIO_NODISCARD_ERROR /* nothing */
+#endif
+
 
 // OIIO_NO_SANITIZE_ADDRESS can be used to mark a function that you don't
 // want address sanitizer to catch. Only use this if you know there are

src/jpeg.imageio/jpegoutput.cpp

@@ -573,24 +573,25 @@ JpgOutput::copy_image(ImageInput* in)
         ImageSpec in_spec;
         ImageSpec config_spec;
         config_spec.attribute("_jpeg:raw", 1);
-        in->open(in_name, in_spec, config_spec);
+        if (!in->open(in_name, in_spec, config_spec)) {
+            errorfmt("{}", in->geterror());
+            return false;
+        }
 
         // Re-open the output
         std::string out_name    = m_filename;
         ImageSpec orig_out_spec = spec();
         close();
         m_copy_coeffs       = (jvirt_barray_ptr*)jpg_in->coeffs();
         m_copy_decompressor = &jpg_in->m_cinfo;
-        open(out_name, orig_out_spec);
-
+        bool ok             = open(out_name, orig_out_spec);
         // Strangeness -- the write_coefficients somehow sets things up
         // so that certain writes only happen in close(), which MUST
         // happen while the input file is still open.  So we go ahead
         // and close() now, so that the caller of copy_image() doesn't
         // close the input file first and then wonder why they crashed.
         close();
-
-        return true;
+        return ok;
     }
 
     return ImageOutput::copy_image(in);

src/libOpenImageIO/imageioplugin.cpp

@@ -504,7 +504,7 @@ pvt::catalog_all_plugins(std::string searchpath)
         // ImageInput::create will take a lock of imageio_mutex
         auto inp = ImageInput::create(f.first);
         lock.lock();
-        if (inp->supports("procedural"))
+        if (inp && inp->supports("procedural"))
             procedural_plugins.insert(f.first);
     }
 }

src/python/py_imagebuf.cpp

@@ -270,7 +270,8 @@ ImageBuf_repr_png(const ImageBuf& self)
 
     std::unique_ptr<ImageOutput> out = ImageOutput::create("temp.png",
                                                            &file_vec);
-    out->open("temp.png", altered_spec);
+    if (!out || !out->open("temp.png", altered_spec))
+        return py::bytes();
     self.write(out.get());
     out->close();
 

testsuite/jpeg-corrupt/ref/out-alt5.txt

@@ -0,0 +1,50 @@
+Reading src/corrupt-exif.jpg
+src/corrupt-exif.jpg :  256 x  256, 3 channel, uint8 jpeg
+    SHA-1: 3EA7BAEB0871E9B9BAADF286521423199ACA2401
+    channel list: R, G, B
+    Orientation: 1 (normal)
+    ResolutionUnit: "in"
+    XResolution: 300
+    YResolution: 300
+    jpeg:subsampling: "4:2:0"
+    oiio:ColorSpace: "srgb_rec709_scene"
+Reading src/corrupt-exif-1626.jpg
+src/corrupt-exif-1626.jpg :  256 x  256, 3 channel, uint8 jpeg
+    SHA-1: 6CBB7DB602A67DB300AB28842F20F6DCA02B82B1
+    channel list: R, G, B
+    Orientation: 1 (normal)
+    ResolutionUnit: "in"
+    XResolution: 300
+    YResolution: 300
+    jpeg:subsampling: "4:2:0"
+    oiio:ColorSpace: "srgb_rec709_scene"
+corrupt-icc-4551.jpg
+iconvert ERROR copying "src/corrupt-icc-4551.jpg" to "out-4551.jpg" :
+	JPEG error: Corrupt JPEG data: bad Huffman code ("src/corrupt-icc-4551.jpg")
+jpeg image resolution must be at least 1x1, but the file specified 0x0x1, 3 chans. Possible corrupt input?
+Reading src/corrupt-icc-4552.jpg
+src/corrupt-icc-4552.jpg : 1500 x 1000, 3 channel, uint8 jpeg
+    SHA-1: 58BC613FDC7513B4F3854D8D2910040B60FF7A7F
+    channel list: R, G, B
+    ICCProfile: 0, 0, 12, 72, 76, 105, 110, 111, 2, 16, 0, 0, 109, 110, 116, 114, ... [3144 x uint8]
+    ResolutionUnit: "in"
+    XResolution: 72
+    YResolution: 72
+    ICCProfile:attributes: "Transparency , Glossy, Positive, Color"
+    ICCProfile:cmm_type: 1281977967
+    ICCProfile:color_space: "RGB"
+    ICCProfile:creation_date: "1998:02:09 06:49:00"
+    ICCProfile:creator_signature: "48502020"
+    ICCProfile:device_class: "Display device profile"
+    ICCProfile:flags: "Not Embedded, Independent"
+    ICCProfile:manufacturer: "49454320"
+    ICCProfile:model: "73524742"
+    ICCProfile:platform_signature: "Microsoft Corporation"
+    ICCProfile:profile_connection_space: "XYZ"
+    ICCProfile:profile_size: 3144
+    ICCProfile:profile_version: "2.1.0"
+    ICCProfile:rendering_intent: "Unknown"
+    jpeg:subsampling: "4:2:0"
+    oiio:ColorSpace: "srgb_rec709_scene"
+iinfo ERROR: "src/corrupt-iptc-8011.jpg" : JPEG error: Corrupt JPEG data: 256 extraneous bytes before marker 0xdb ("src/corrupt-iptc-8011.jpg")
+Corrupted IPTC data