More tests for PNG code

[piponazo]

Changes:

• Added pngchunk_int.hpp to the list of sources in CMake (helps to the code navigation in IDEs).
• Added new tests for PngChunk and PngImage.
• Improvement in PngChunk::keyTXTChunk.
• Initialise some unitialised variables.
• clang-format in pngchunk_int.cpp
• Minimum C++ Standard upgraded

[codecov]

Codecov Report

Merging #2051 (a18a18e) into main (2493e55) will increase coverage by 0.05%.
The diff coverage is 73.70%.

❗ Current head a18a18e differs from pull request most recent head 846b7f0. Consider uploading reports for the commit 846b7f0 to get more accurate results

@@            Coverage Diff             @@
##             main    #2051      +/-   ##
==========================================
+ Coverage   61.43%   61.48%   +0.05%     
==========================================
  Files          96       96              
  Lines       19203    19192      -11     
  Branches     9847     9840       -7     
==========================================
+ Hits        11797    11801       +4     
+ Misses       5087     5077      -10     
+ Partials     2319     2314       -5     

Impacted Files	Coverage Δ
src/jpgimage.cpp	70.48% <ø> (ø)
src/pngimage.cpp	63.87% <60.00%> (+3.76%)	⬆️
src/pngchunk_int.cpp	72.85% <73.94%> (-0.75%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2493e55...846b7f0. Read the comment docs.

[clanmills]

Well done for investigating this code, @piponazo. I've never liked that code. I suspect the header parser is not robust.

I documented PNG in my book. https://clanmills.com/exiv2/book/#PNG

In my book, I've said:

The signatures: "Raw profile type iptc" and "Raw profile type exif" introduce a compressed block which when expanded is an ascii string with the following format. The number is count of hex code bytes. This number is redundant and does not need to be decoded.

\n
exif\n
    number\n
hexEncodedBinary\n
....

I encourage you to read the PNG spec to verify that this is correct. Then carefully review the code that parses this strange entity. The tvisitor.cpp code in my book does not parse this.

[piponazo]

I encourage you to read the PNG spec to verify that this is correct. Then carefully review the code that parses this strange entity. The tvisitor.cpp code in my book does not parse this.

I actually did some time ago. I was reviewing my notes and I remembered about #147 and #1505. I would like to spend some more time reviewing the Exiv2 code base and understanding the main components before taking the adventure of addressing such an important addition. My way to understand the code is to add "Characterisation tests" as I look at different parts of the code. Afterwards, if I think I can do some improvement in the code, I have a safety net that will allow me to make changes without fearing to break things.

[hassec]

LGTM 👍

Footnote about the formatting, though:
did you use the clang-format.optional config for this?
I think we should make sure that there is one common truth to the formatting, and maybe it's even worth introducing a formatting check in GHA, otherwise it's not possible to enforce it.
Of course, that would require one global reformat first...
So def. not for this PR, but we should really avoid risking a formatting back and forth fight between different configs as that creates annoying diffs.

[piponazo]

did you use the clang-format.optional config for this?

Yes, I used that one. We have also discussed in the past about the possibility of doing a global re-format of the project, but we did not do it until now due to the complications it would add to port changes from 0.27-maintenance to main. But I am also in favour of doing that global re-formatting at some point and adding something in CI to enforce the correct formatting of code.