pnggroup / libpng

LIBPNG: Portable Network Graphics support, official libpng repository
http://libpng.sf.net
Other
1.25k stars 611 forks source link

Feature Request - graceful error returns for reading/writing images #532

Open d3x0r opened 7 months ago

d3x0r commented 7 months ago

I've run into issues where libpng read a bad image and triggered an abort, rather than letting the application gracefully handle it. But then I'm no fan of setjmp/longjmp so maybe it's my refusal to use those that's the issue.

The error path for reading/writing png files isn't exactly that long.... that is - it's not terribly complex to catch if( !somefunction ) return false; and really doesn't anything as esoteric as longjmp to get out of.

pngwrite.c, pngwio.c, pngrio.c pngread.c, pngpread.c have a few functions that can have their return type changed from 'void' to 'int' (or boolean) and return true/false status instead of triggering abort through pngerror.c.

This is basically my applied change for libpng 1.6.40 to add 'PNG_GRACEFUL_ERROR' option which allows errors to go back to the application to handle them gracefully. https://github.com/d3x0r/SACK/commit/033a9b2262c3a37eb8fa1c00fcda835345d02b5f (ignore the CMakeLists.part and pnglibconf.h additions which I end up tracking )

jbowler commented 7 months ago

This is what I want to see for libpng-ng; i.e. a rewrite which uses a full error code scheme, either 'true/false' with separate error codes or segmented error codes (like in Apollo's Aegis, which used 32-bit codes throughout the OS and still remains the best designed OS I've ever used.)

A compatibility layer can be built on this at the API level; APIs call an appropriate set of libpng-ng APIs and translate the error numbers to error codes.

Meanwhile in older versions of libpng (i.e. 1.6 and 1.5) the simplified API has existed to solve this problem (and other problems of general utility) and works

It isn't just setjmp/longjmp. It's also the reliance on callback APIs which have to be blocking (IO and memory). That's why the whole progressive reader code duplication came into existence and that needs fixing in libpng-ng as well. Notice that Mark Adler's zlib implementation is non-blocking and has an API which can be adopted within libpng-ng without itself precluding the use of other LZ77 implementations.

jbowler commented 7 months ago

@d3xor I suggest you launch a discussion on png-mng-implement.

jbowler commented 7 months ago

@d3x0r one of my current points of focus is the extent to which application and library developers find the simplified API inadequate. In view of this you are the perfect person for comment: why isn't the simplified API sufficient for your needs?

d3x0r commented 7 months ago

The code using libpng dates back to the late 90's. the simplified API might be sufficient... The fixes to the library layer were driven by 'the usage has always worked' and for some reason the library generated abort() out of the blue. I don't think the reader is even paying attention to error codes; either the image worked or it doesn't. I occasionally used the writer code for debugging like font tile sheets built for opengl output; but usually I'm not even doing writes.

jbowler commented 7 months ago

Thanks for the information. png_error can always be produced on 'read' by broken input - the simplified API is the only way at present round that but it doesn't do everything the other reader APIs do. On 'write' abort() is just fine because png_error should only result from internal libpng errors or application errors (including an explicit call to png_error in a callback.)