Skip to content

Fix checksum doesn't match error #923

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

Merged
merged 2 commits into from
Mar 18, 2024
Merged

Fix checksum doesn't match error #923

merged 2 commits into from
Mar 18, 2024

Conversation

umbynos
Copy link
Contributor

@umbynos umbynos commented Mar 15, 2024

Please check if the PR fulfills these requirements

  • The PR has no duplicates (please search among the Pull Requests
    before creating one)
  • Tests for the changes have been added (for bug fixes / features)
  • What kind of change does this PR introduce?

Bug Fix

  • What is the current behavior?

Close #920

  • What is the new behavior?

The checksum is properly verified, keeping into account the padding that could be present in the archives

  • Does this PR introduce a breaking change?

no

  • Other information:

@umbynos umbynos marked this pull request as draft March 15, 2024 13:49
@umbynos umbynos self-assigned this Mar 15, 2024
@umbynos umbynos added the type: imperfection Perceived defect in any part of project label Mar 15, 2024
The buffer was not containing the full archive because when red by `extract.Archive(...)` some bytes were skipped because of padding.
This was causing the checksum to be different from the expected one
@umbynos umbynos force-pushed the fix-checksum-error branch from d787bea to 353057e Compare March 15, 2024 14:30
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 78.57143% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 20.78%. Comparing base (dafef3c) to head (353057e).
Report is 5 commits behind head on main.

Files Patch % Lines
v2/pkgs/tools.go 78.57% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #923      +/-   ##
==========================================
- Coverage   20.94%   20.78%   -0.17%     
==========================================
  Files          43       43              
  Lines        3161     3200      +39     
==========================================
+ Hits          662      665       +3     
- Misses       2403     2436      +33     
- Partials       96       99       +3     
Flag Coverage Δ
unit 20.78% <78.57%> (-0.17%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@per1234 per1234 added the topic: code Related to content of the project itself label Mar 15, 2024
@umbynos umbynos marked this pull request as ready for review March 18, 2024 08:38
Copy link
Contributor

@alessio-perugini alessio-perugini left a comment

Choose a reason for hiding this comment

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

🤯 this will add a bit of overhead memory-wise. If we see some users pointing out memory-related problems it might be this. However, I don't expect this to be a problem as most of the tools are very small in size.
Well done 🙌

@umbynos umbynos merged commit 236fab6 into main Mar 18, 2024
42 checks passed
@umbynos umbynos deleted the fix-checksum-error branch March 18, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: imperfection Perceived defect in any part of project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Installing avrdude 6.3.0-arduino17 fails with checksum doesn't match
5 participants