@asmorkalov, I see that tests are failing due to old GitHub dependencies. We probably want a dependabot config. This is what we have for AVIF:https://github.com/AOMediaCodec/libavif/blob/main/.github/dependabot.yml

It will make PRs once a month to update your CI files (if needed)

@vrabaud Can you elaborate on this a bit more? IMHO there is nothing wrong with the original code.

@vrabaud Could you bring more details. I do not see any difference.

My code was wrong, the new one works. Temporary variables do not play well withsetjmp ...

It's still a bit inconsistent to me. I'm aware that setjmp/longjmp has the potential to bypass destructors (and thereby causing memory leaks) in certain situations. If this is a problem in 'processing_start' (not sure if it is), wouldn't 'readHeader' suffer from the same issue? So I think either local 'PngPtrs' should be avoided in both cases or in none of them. Also, if we go the route of working directly with 'm_png_ptrs', we should probably clear it in case of failures. Like it is done in 'processing_finish'.

There is another small thing I find unfortunate: The default constructor of 'PngPtrs' allocates data. That means, when a PngDecoder is instantiated, we allocate stuff that gets thrown away on the very first usage of this decoder instance. I guess it's better to add an explicit 'init' method to 'PngPtrs' and let the default constructor just zero-initialize all member variables.

@warped-rudi , I indeed refactored to be more consistent. The READ memory access is fixed by the extrasetjmp in readData`.

I added a fix forhttps://oss-fuzz.com/testcase-detail/5048650127966208

That look good to me! Two more nit-picks: PngDecoder (as well as PngEncoder) don't have to have protected members. Make them private. Also the question wether or not to call ClearPngPtr() every time setjmp() returns a non-zero value persists. Currently it's only done in processing_finish().

Linux builds report:

/home/ci/opencv/modules/imgcodecs/src/grfmt_png.cpp:540:21: warning: variable 'buffer' might be clobbered by 'longjmp' or 'vfork' [-Wclobbered]

I fixed the two comments.

That look good to me! Two more nit-picks: PngDecoder (as well as PngEncoder) don't have to have protected members. Make them private. Also the question wether or not to call ClearPngPtr() every time setjmp() returns a non-zero value persists. Currently it's only done in processing_finish().

ClearPngPtr does not need to be called every time setjmp returns a non-zero as it deals with variables outside of the longjmp. Those variables will be cleared normally by the destructor. I removed it from processing_finish.