rpng icon indicating copy to clipboard operation
rpng copied to clipboard

No error handling

Open edubart opened this issue 2 years ago • 3 comments

I was considering using the library for reading/saving PNG, because of the additional chunks abstraction API, but I've noticed a major design decision that disallows me using it:

  1. Some functions doesn't return an error code, instead they fail silently, so there is not way to handle errors, like rpng_save_image.
  2. Corrupt invalid/images may cause out of bounds error and then a crash, because the functions doesn't receive the buffer size

I have read this statement:

Note an important detail: memory functions do not receive the size of the buffer. It was a design decision. Data is validated following PNG specs (png magic number, chunks data, IEND closing chunk) but it's expected that user provides valid data.

I think this library should reconsider this in the future, to be usable for example in production software, where hard disk corruption and write failures is a possibility that we would like to handle the error.

edubart avatar Dec 24 '21 11:12 edubart

I'm afraid library has not been tested enough yet for production but I'm pleased if anyone is willing to give it a try and provide some feedback.

I think most functions output some kind of log message in case of warnings/errors but that's definitely a point for improvement. rpng_load_image() returns NULL in case of failing, what additional information would you need?

I think this library should reconsider this in the future, to be usable for example in production software, where hard disk corruption and write failures is a possibility that we would like to handle the error.

First implementation of the library is more than 1 year old and it was originally created for a personal need in some of my tools (basically, add custom chunks of data) but lately I reviewed it to make it more general purpose. I can reconsider some of the original decisions.

EDIT: Related info: https://www.w3.org/TR/2003/REC-PNG-20031110/#13Decoders.Errors

raysan5 avatar Dec 24 '21 17:12 raysan5

what additional information would you need?

I meant functions like rpng_save_image, rpng_chunk_write_text, etc to return an error code, instead of nothing (void), so we can know if the operation completed with success. Just logging is usually not enough because we can't handle the error programmatically.

EDIT: Related info:

Validating just PNG format is usually not safe enough, because for example if somehow a previously valid PNG file is corrupt by truncating its last bytes, the library will access memory out of the buffer bounds and then we may have a crash.

edubart avatar Dec 24 '21 18:12 edubart

Function could returns several error code values to provide some diagnostic functionality. Some possible values are:

  • RPNG_SUCCESS - Image loaded/saved successfully.
  • RPNG_ERROR_FILE_NOT_FOUND - The requested file can not be opened (spelling issues, file actually does not exist...)
  • RPNG_ERROR_PIXEL_FORMAT - Not a supported PNG image format. Currently indexed color not supported.
  • RPNG_ERROR_MEMORY_ALLOC - Memory could not be allocated for operation.

raysan5 avatar Mar 23 '22 12:03 raysan5