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

Clarify ownership model of CiffComponent::pData_ #1930

Merged

Conversation

kevinbackhouse
Copy link
Collaborator

This is a stepping stone to some other changes that I would like to make to DataBuf. In particular, I would like to get rid of the release method and instead use std::move for ownership transfers of DataBuf.

This changes simplifies the ownership model of CiffComponent::pData_. It is usually a pointer into read-only memory owned by somebody else, but when isAllocated_ == true, it owns the memory. This, combined with the fact that we sometimes do arithmetic on the pointer, makes me nervous. I have changed it so that pData_ always points to data owned by somebody else. I added a field named storage_ for when CiffComponent needs to own the data itself.

@kevinbackhouse kevinbackhouse added this to the v1.00 milestone Sep 26, 2021
@codecov
Copy link

codecov bot commented Sep 26, 2021

Codecov Report

Merging #1930 (3b0d827) into main (774e662) will increase coverage by 0.24%.
The diff coverage is 80.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1930      +/-   ##
==========================================
+ Coverage   60.87%   61.12%   +0.24%     
==========================================
  Files          96       96              
  Lines       19041    19055      +14     
  Branches     9726     9743      +17     
==========================================
+ Hits        11591    11647      +56     
+ Misses       5138     5092      -46     
- Partials     2312     2316       +4     
Impacted Files Coverage Δ
src/crwimage_int.hpp 84.61% <71.42%> (+0.24%) ⬆️
src/crwimage_int.cpp 74.07% <100.00%> (+0.03%) ⬆️
src/makernote_int.cpp 67.62% <0.00%> (-0.72%) ⬇️
src/exiv2.cpp 57.85% <0.00%> (ø)
src/futils.cpp 72.06% <0.00%> (ø)
src/properties.cpp 73.36% <0.00%> (ø)
src/pentaxmn_int.cpp 72.92% <0.00%> (+0.23%) ⬆️
src/convert.cpp 53.84% <0.00%> (+0.34%) ⬆️
src/value.cpp 73.18% <0.00%> (+0.46%) ⬆️
src/tags_int.cpp 77.05% <0.00%> (+0.71%) ⬆️
... and 2 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 774e662...3b0d827. Read the comment docs.

src/crwimage_int.cpp Outdated Show resolved Hide resolved
@kevinbackhouse
Copy link
Collaborator Author

Thanks @neheb!

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