Skip to content

Fix nasty append bug that caused 'min' namespace id modifications #30

Merged
liamsi merged 1 commit intomasterfrom
hlib/digest-bytes-bug
Mar 29, 2021
Merged

Fix nasty append bug that caused 'min' namespace id modifications #30
liamsi merged 1 commit intomasterfrom
hlib/digest-bytes-bug

Conversation

@Wondertan
Copy link
Member

@Wondertan Wondertan commented Mar 29, 2021

@Wondertan Wondertan requested a review from liamsi March 29, 2021 15:28
@Wondertan Wondertan self-assigned this Mar 29, 2021
@codecov
Copy link

codecov bot commented Mar 29, 2021

Codecov Report

Merging #30 (2198e0e) into master (babfaf9) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
+ Coverage   94.70%   94.77%   +0.06%     
==========================================
  Files           7        7              
  Lines         302      306       +4     
==========================================
+ Hits          286      290       +4     
  Misses         11       11              
  Partials        5        5              
Impacted Files Coverage Δ
namespace/digest.go 100.00% <100.00%> (ø)

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 babfaf9...2198e0e. Read the comment docs.

Comment on lines +43 to +47
return append(append(append(
make([]byte, 0, len(d.min)*2+len(d.digest)),
d.min...),
d.max...),
d.digest...)
Copy link
Member

@liamsi liamsi Mar 29, 2021

Choose a reason for hiding this comment

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

@adlerjohn will love this one 🤣

Note that differently from the title, the nid does not get modified. Still the race detector would raise an alarm here before as this is seen as a WRITE.

Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

👏🏼 Thanks! Did you verify that this actually fixes the problem with the race detector in ll-core?

@liamsi liamsi merged commit ddcc720 into master Mar 29, 2021
@liamsi liamsi deleted the hlib/digest-bytes-bug branch March 29, 2021 19:20
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