Add OIIO_NODISCARD to ImageInput create/open/read methods

src/include/OpenImageIO/imageio.h

@@ -1051,16 +1051,18 @@ class OIIO_API ImageInput {
     ///         A `unique_ptr` that will close and free the ImageInput when
     ///         it exits scope or is reset. The pointer will be empty if the
     ///         required writer was not able to be created.
-    static unique_ptr create (string_view filename, bool do_open=false,
-                              const ImageSpec *config=nullptr,
-                              Filesystem::IOProxy* ioproxy = nullptr,
-                              string_view plugin_searchpath = "");
+    OIIO_NODISCARD static unique_ptr create (string_view filename,
+                                             bool do_open=false,
+                                             const ImageSpec *config=nullptr,
+                                             Filesystem::IOProxy* ioproxy = nullptr,
+                                             string_view plugin_searchpath = "");
 
     /// Create an ImageInput using a UTF-16 encoded wstring filename.
-    static unique_ptr create (const std::wstring& filename, bool do_open=false,
-                              const ImageSpec *config=nullptr,
-                              Filesystem::IOProxy* ioproxy = nullptr,
-                              string_view plugin_searchpath = "") {
+    OIIO_NODISCARD static unique_ptr create (const std::wstring& filename,
+                                             bool do_open=false,
+                                             const ImageSpec *config=nullptr,
+                                             Filesystem::IOProxy* ioproxy = nullptr,
+                                             string_view plugin_searchpath = "") {
         return create(Strutil::utf16_to_utf8(filename), do_open, config,
                       ioproxy, plugin_searchpath);
     }
@@ -1180,10 +1182,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 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 bool open (const std::wstring& name, ImageSpec &newspec) {
         return open(Strutil::utf16_to_utf8(name), newspec);
     }
 
@@ -1207,13 +1210,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 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 bool open (const std::wstring& name, ImageSpec &newspec,
+                              const ImageSpec& config OIIO_MAYBE_UNUSED) {
         return open(name,newspec);
     }
 
@@ -1411,15 +1415,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 +1438,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 +1486,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 +1511,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 +1781,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 +1912,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);
 
     /// @}
 
@@ -4695,6 +4703,6 @@ OIIO_NAMESPACE_END
 
 #if FMT_VERSION >= 100000
 FMT_BEGIN_NAMESPACE
-template<> struct formatter<OIIO::ROI> : ostream_formatter {};
+template<> struct formatter<OIIO::ROI> : ostream_formatter { };
 FMT_END_NAMESPACE
 #endif

src/include/OpenImageIO/platform.h

@@ -461,6 +461,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

@@ -33,9 +33,7 @@ class JpgOutput final : public ImageOutput {
     ~JpgOutput() override { close(); }
     const char* format_name(void) const override { return "jpeg"; }
     int supports(string_view feature) const override
-    {
-        return (feature == "exif" || feature == "iptc" || feature == "ioproxy");
-    }
+    { return (feature == "exif" || feature == "iptc" || feature == "ioproxy"); }
     bool open(const std::string& name, const ImageSpec& spec,
               OpenMode mode = Create) override;
     bool write_scanline(int y, int z, TypeDesc format, const void* data,
@@ -104,9 +102,7 @@ OIIO_PLUGIN_EXPORTS_BEGIN
 
 OIIO_EXPORT ImageOutput*
 jpeg_output_imageio_create()
-{
-    return new JpgOutput;
-}
+{ return new JpgOutput; }
 
 OIIO_EXPORT const char* jpeg_output_extensions[]
     = { "jpg", "jpe", "jpeg", "jif", "jfif", "jfi", nullptr };
@@ -573,24 +569,23 @@ 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))
+            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

@@ -154,9 +154,7 @@ ImageBuf_setpixel(ImageBuf& buf, int x, int y, int z, py::object p)
 
 void
 ImageBuf_setpixel2(ImageBuf& buf, int x, int y, py::object p)
-{
-    ImageBuf_setpixel(buf, x, y, 0, p);
-}
+{ ImageBuf_setpixel(buf, x, y, 0, p); }
 
 
 void
@@ -194,16 +192,12 @@ ImageBuf_get_pixels(const ImageBuf& buf, TypeDesc format, ROI roi = ROI::All())
 void
 ImageBuf_set_deep_value(ImageBuf& buf, int x, int y, int z, int c, int s,
                         float value)
-{
-    buf.set_deep_value(x, y, z, c, s, value);
-}
+{ buf.set_deep_value(x, y, z, c, s, value); }
 
 void
 ImageBuf_set_deep_value_uint(ImageBuf& buf, int x, int y, int z, int c, int s,
                              uint32_t value)
-{
-    buf.set_deep_value(x, y, z, c, s, value);
-}
+{ buf.set_deep_value(x, y, z, c, s, value); }
 
 
 
@@ -270,7 +264,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();