Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use more iterators with copy_n #3209

Merged
merged 1 commit into from
Mar 14, 2025
Merged

use more iterators with copy_n #3209

merged 1 commit into from
Mar 14, 2025

Conversation

neheb
Copy link
Collaborator

@neheb neheb commented Mar 13, 2025

It's a bit safer to do so.

@neheb neheb force-pushed the 3 branch 2 times, most recently from 29af3b8 to 77ee2b0 Compare March 13, 2025 03:02
@neheb neheb requested review from kevinbackhouse and kmilos March 13, 2025 19:03
src/image.cpp Outdated
DataBuf buf(allocate64); // allocate a buffer
std::copy_n(dir.c_data(8), 4, buf.begin()); // copy dir[8:11] into buffer (short strings)
DataBuf buf(allocate64); // allocate a buffer
std::copy(dir.begin() + 8, dir.begin() + 8 + 4, buf.begin()); // copy dir[8:11] into buffer (short strings)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this better? It seems more verbose than the old version that used std::copy_n.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably makes more sense to use iterators in general instead of C pointers with these functions.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reworked.

It's a bit safer to do so.

Signed-off-by: Rosen Penev <[email protected]>
Copy link

codecov bot commented Mar 14, 2025

Codecov Report

Attention: Patch coverage is 71.42857% with 52 lines in your changes missing coverage. Please review.

Project coverage is 63.91%. Comparing base (ba0d5ea) to head (9f3dfc9).
Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
src/webpimage.cpp 60.29% 25 Missing and 2 partials ⚠️
src/jp2image.cpp 74.28% 8 Missing and 1 partial ⚠️
src/rafimage.cpp 61.53% 0 Missing and 5 partials ⚠️
src/pgfimage.cpp 57.14% 2 Missing and 1 partial ⚠️
src/pngimage.cpp 77.77% 0 Missing and 2 partials ⚠️
src/gifimage.cpp 75.00% 0 Missing and 1 partial ⚠️
src/jpgimage.cpp 87.50% 0 Missing and 1 partial ⚠️
src/makernote_int.cpp 92.85% 1 Missing ⚠️
src/mrwimage.cpp 80.00% 0 Missing and 1 partial ⚠️
src/preview.cpp 0.00% 1 Missing ⚠️
... and 1 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3209      +/-   ##
==========================================
- Coverage   63.93%   63.91%   -0.02%     
==========================================
  Files         104      104              
  Lines       21755    21752       -3     
  Branches    10636    10636              
==========================================
- Hits        13908    13903       -5     
- Misses       5639     5641       +2     
  Partials     2208     2208              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@neheb neheb merged commit cfb0475 into Exiv2:main Mar 14, 2025
61 checks passed
@neheb neheb deleted the 3 branch March 14, 2025 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants