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.

Sign up for GitHub

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

Jump to bottom

clang-tidy with headers #2200

Merged
merged 9 commits into from
Apr 10, 2022
Merged

clang-tidy with headers #2200

merged 9 commits into from
Apr 10, 2022

Conversation

neheb
Copy link
Collaborator

@neheb neheb commented Apr 10, 2022

No description provided.

@lgtm-com
Copy link

lgtm-com bot commented Apr 10, 2022

This pull request introduces 2 alerts and fixes 2 when merging cd1ce62 into 053f516 - view on LGTM.com

new alerts:

  • 2 for 'new[]' array freed with 'delete'

fixed alerts:

  • 2 for Inconsistent definition of copy constructor and assignment ('Rule of Two')

@codecov
Copy link

codecov bot commented Apr 10, 2022 •
edited
Loading

Codecov Report

Merging #2200 (c997b09) into main (053f516) will increase coverage by 0.01%.
The diff coverage is 87.82%.

@@            Coverage Diff             @@
##             main    #2200      +/-   ##
==========================================
+ Coverage   63.31%   63.33%   +0.01%     
==========================================
  Files          99      115      +16     
  Lines       19586    19607      +21     
  Branches     9555     9552       -3     
==========================================
+ Hits        12401    12418      +17     
- Misses       5113     5115       +2     
- Partials     2072     2074       +2
Impacted Files Coverage Δ
include/exiv2/datasets.hpp 100.00% <ø> (ø)
include/exiv2/epsimage.hpp 0.00% <0.00%> (ø)
include/exiv2/image.hpp 100.00% <ø> (ø)
include/exiv2/properties.hpp 100.00% <ø> (ø)
include/exiv2/tags.hpp 100.00% <ø> (ø)
include/exiv2/types.hpp 76.59% <ø> (ø)
src/basicio.cpp 51.44% <ø> (ø)
src/cr2header_int.hpp 33.33% <ø> (ø)
src/makernote_int.hpp 92.30% <ø> (ø)
src/orfimage_int.hpp 100.00% <ø> (ø)
... and 42 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 053f516...c997b09. Read the comment docs.

piponazo
piponazo reviewed Apr 10, 2022
View reviewed changes
src/crwimage.cpp Outdated
@@ -92,10 +92,10 @@ void CrwImage::writeMetadata() {
CrwParser::encode(blob, buf.c_data(), buf.size(), this);

// Write new buffer to file
auto tempIo = std::make_unique<MemIo>();
tempIo->write((!blob.empty() ? &blob[0] : nullptr), blob.size());
auto tempIo = MemIo();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good catchs in this commit 👏 . Is this something automatically detected by clang-tidy?

I would only suggest to take a step more here. I think it is more intuitive to write MemIo tempIo; (Here and in all the other cases)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Nope. Saw it while browsing the code.

@lgtm-com
Copy link

lgtm-com bot commented Apr 10, 2022

This pull request fixes 2 alerts when merging 0ae5280 into 053f516 - view on LGTM.com

fixed alerts:

  • 2 for Inconsistent definition of copy constructor and assignment ('Rule of Two')

@lgtm-com
Copy link

lgtm-com bot commented Apr 10, 2022

This pull request fixes 2 alerts when merging eead935 into 053f516 - view on LGTM.com

fixed alerts:

  • 2 for Inconsistent definition of copy constructor and assignment ('Rule of Two')

@neheb
remove some unique_ptrs
0d971e9 
They're not really used as pointers.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
@neheb
clang-tidy: default member init
1f05448 
Signed-off-by: Rosen Penev <rosenp@gmail.com>
@neheb
clang-tidy: replace throw() with noexcept
e654b29 
Signed-off-by: Rosen Penev <rosenp@gmail.com>
@neheb
clang-tidy: use delete
49bf7e1 
Signed-off-by: Rosen Penev <rosenp@gmail.com>
@neheb
clang-tidy: use nullptr
ae66ece 
Signed-off-by: Rosen Penev <rosenp@gmail.com>
@neheb
clang-tidy: manual clang-tidy fixes
a20ace2 
clang-tidy has issues applying these.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
@neheb
clang-tidy: manual special functions
b53ed72 
Signed-off-by: Rosen Penev <rosenp@gmail.com>
@neheb
replace malloc/free with new/delete
d789968 
Signed-off-by: Rosen Penev <rosenp@gmail.com>
@neheb
algorithm conversions
c997b09 
Signed-off-by: Rosen Penev <rosenp@gmail.com>
@lgtm-com
Copy link

lgtm-com bot commented Apr 10, 2022

This pull request fixes 2 alerts when merging c997b09 into 053f516 - view on LGTM.com

fixed alerts:

  • 2 for Inconsistent definition of copy constructor and assignment ('Rule of Two')

piponazo
piponazo approved these changes Apr 10, 2022
View reviewed changes
Copy link
Collaborator

@piponazo piponazo left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks!

@piponazo piponazo merged commit 84ba579 into Exiv2:main Apr 10, 2022
@nehaljwani nehaljwani mentioned this pull request Apr 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@piponazo piponazo piponazo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@neheb @piponazo