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

Modernizations and strictness improvements #464

Merged
merged 17 commits into from
Feb 21, 2023
Merged

Modernizations and strictness improvements #464

merged 17 commits into from
Feb 21, 2023

Conversation

cgzones
Copy link
Contributor

@cgzones cgzones commented Feb 21, 2023

  • modernize the codebase by using modern C++ features
  • be more const-correct by using adjusting casts and enable -Wcast-qual
  • declare more read-only functions const
  • avoid value truncations in multiple places
  • avoid out-of-bounds memory access and integer overflows on malicious ELF data

Enable to parse the header file on its own, e.g. for language servers
(clangd).
Make the behavior of functions more explicit.
Avoid creating a stringstream by using find().
Also declare the function static and warn if return value not used.
Prevent overflows in the addtion of q and size, and avoid truncations in
callers by using size_t as type for size.
Enable common additional compiler warnings.
Abort on truncation of values being written to the ELF data, to prevent
silent behavior mismatch.
Use auto to avoid implicit type conversion, hiding possible value
truncation.
Reject ELF data that would lead to invalid memory access or integer
overflows.
@Mic92 Mic92 marked this pull request as ready for review February 21, 2023 19:52
@@ -196,11 +196,11 @@ static FileContents readFile(const std::string & fileName,
while ((portion = read(fd, contents->data() + bytesRead, size - bytesRead)) > 0)
bytesRead += portion;

close(fd);
Copy link
Member

Choose a reason for hiding this comment

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

This could be even autoclosed later on.

@Mic92
Copy link
Member

Mic92 commented Feb 21, 2023

Thank. Very appreciated. bors merge

@Mic92
Copy link
Member

Mic92 commented Feb 21, 2023

bors merge

@bors bors bot merged commit e37f892 into NixOS:master Feb 21, 2023
@cgzones cgzones deleted the work branch February 22, 2023 16:24
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.

None yet

2 participants