Skip to content

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
@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