More bounds checking in Adjust::adjustDateTime

[kevinbackhouse]

I moved #2231 to draft because it had some build failures on Windows, caused by the code in Adjust::adjustDateTime. The problem is that struct tm has fields of type int, which means that there's quite a lot of implicit casting happening here.

In this PR, I've added some bounds checking, static_cast, and Safe::add, to make the conversions explicit. After this PR is merged, I'll be able to fix the build failures in #2231 more easily.

[codecov]

Codecov Report

Merging #2244 (2e0ab1a) into main (19dc566) will increase coverage by 0.03%.
The diff coverage is 95.65%.

@@            Coverage Diff             @@
##             main    #2244      +/-   ##
==========================================
+ Coverage   63.42%   63.45%   +0.03%     
==========================================
  Files         117      118       +1     
  Lines       19593    19608      +15     
  Branches     9551     9561      +10     
==========================================
+ Hits        12426    12442      +16     
  Misses       5098     5098              
+ Partials     2069     2068       -1     

Impacted Files	Coverage Δ
app/actions.cpp	65.01% <95.65%> (+0.34%)	⬆️
app/actions.hpp	100.00% <0.00%> (ø)
app/exiv2app.hpp	100.00% <0.00%> (ø)
src/nikonmn_int.cpp	57.10% <0.00%> (ø)
src/crwimage_int.hpp	93.02% <0.00%> (ø)
src/makernote_int.hpp	92.30% <0.00%> (ø)
src/tiffimage_int.cpp	77.90% <0.00%> (ø)
include/exiv2/value.hpp	84.44% <0.00%> (ø)
include/exiv2/webpimage.hpp	100.00% <0.00%> (ø)
src/tiffcomposite_int.cpp	75.50% <0.00%> (+0.02%)	⬆️
... and 1 more

---

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 19dc566...2e0ab1a. Read the comment docs.

[piponazo]

Thanks for taking care of this.

At first sight all the additions look good, but I am always a bit skeptical with changes not providing its own tests to validate the implementation. Since these changes are in the application level, you cannot add simple tests in the unitTests target. However, I think it should be possible to add some integration tests in the python suite to exercise this new code.

I let you decide if you want to address such task in this PR or create a new issue describing the lack of tests for this new code, so that somebody else (or you in the future) want to take care of that.

Pull request has been modified.

[kevinbackhouse]

@piponazo: thanks for the review! I added a new test, which triggers 6 of the 8 new error messages. I don't believe it's currently possible to trigger the "seconds" error message because I can't find a command-line argument that will let me enter a large number for the seconds adjustment.

[kevinbackhouse]

The test was failing on Windows (due to the different size of the long type), so I've removed it from this PR and transferred it to #2231. The test passes on all platforms after the switch to int64_t in #2231.

[piponazo]

Great! Thanks for the extra effort 💪