Skip to content

Improve: [CodeFactor] eliminate (some of) our C style casts#5355

Merged
SlySven merged 3 commits intoMudlet:developmentfrom
SlySven:CodeFactor_eliminateOurCStyleCasts
Oct 9, 2021
Merged

Improve: [CodeFactor] eliminate (some of) our C style casts#5355
SlySven merged 3 commits intoMudlet:developmentfrom
SlySven:CodeFactor_eliminateOurCStyleCasts

Conversation

@SlySven
Copy link
Copy Markdown
Member

@SlySven SlySven commented Aug 8, 2021

These have no place in C++ code!

Unfortunately there are some in third party bits which can't be fixed here but should be raised upstream.

This should help with #1650.

Signed-off-by: Stephen Lyons slysven@virginmedia.com

Release post highlight

Probably none - I'm not sure that this is something that we want to make a song-and-dance about given that it doesn't totally eliminate the issue!

These have no place in C++ code!

Unfortunately there are some in third party bits which can't be fixed here
but should be raised upstream.

This should help with Mudlet#1650.

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
@SlySven SlySven requested a review from a team as a code owner August 8, 2021 21:07
@SlySven SlySven requested a review from a team August 8, 2021 21:07
@add-deployment-links
Copy link
Copy Markdown

add-deployment-links bot commented Aug 8, 2021

Hey there! Thanks for helping Mudlet improve. 🌟

Test versions

You can directly test the changes here:

No need to install anything - just unzip and run.
Let us know if it works well, and if it doesn't, please give details.

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/TTrigger.cpp Outdated
return;
}
auto * filterSubject = (char*)malloc(capture.size() + 2048);
auto * filterSubject = reinterpret_cast<char*>(malloc(capture.size() + 2048));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

auto * filterSubject = reinterpret_cast<char*>(malloc(capture.size() + 2048));
                       ^

src/TTrigger.cpp Outdated
return;
}
auto * filterSubject = (char*)malloc(capture.size() + 2048);
auto * filterSubject = reinterpret_cast<char*>(malloc(capture.size() + 2048));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]

auto * filterSubject = reinterpret_cast<char*>(malloc(capture.size() + 2048));
                                               ^

src/ctelnet.cpp Outdated

mZstream.avail_out = 100000;
mZstream.next_out = (Bytef*)out_buffer;
mZstream.next_out = reinterpret_cast<Bytef*>(out_buffer);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]

mZstream.next_out = reinterpret_cast<Bytef*>(out_buffer);
                    ^

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Aug 9, 2021

@vadi2
Copy link
Copy Markdown
Member

vadi2 commented Aug 29, 2021

@SlySven what should we do about this?

@SlySven
Copy link
Copy Markdown
Member Author

SlySven commented Oct 9, 2021

@SlySven what should we do about this?

I'll probably remove the ones that are still confusing me/the clang-tidy bot!

There may be a better way - but it doesn't jump out at me - to deal with
the outstanding ones.

Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

return;
}
auto * filterSubject = (char*)malloc(capture.size() + 2048);
auto * filterSubject = static_cast<char*>(malloc(capture.size() + 2048));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: do not manage memory manually; consider a container or a smart pointer [cppcoreguidelines-no-malloc]

auto * filterSubject = static_cast<char*>(malloc(capture.size() + 2048));
                                          ^

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good idea, but probably outside of the scope of this PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good idea, but probably outside of the scope of this PR.

You mean the "consider a container or a smart pointer [cppcoreguidelines-no-malloc]" is a good idea, right? - I guess it is - but since it is interfacing to a C library I'd be quite happy to leave it to someone else a different PR ...! 😜

@SlySven SlySven changed the title CodeFactor: eliminate our C style casts Improve: [CodeFactor] eliminate (some of) our C style casts Oct 9, 2021
@SlySven
Copy link
Copy Markdown
Member Author

SlySven commented Oct 9, 2021

Retitled to fit our recently discussed PR title patterning. 😀

@SlySven SlySven merged commit cda792c into Mudlet:development Oct 9, 2021
@SlySven SlySven deleted the CodeFactor_eliminateOurCStyleCasts branch October 9, 2021 19:11
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