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

compress/bzip2: dead shifts in newHuffmanTree #17949

Closed
mvdan opened this issue Nov 16, 2016 · 5 comments
Closed

compress/bzip2: dead shifts in newHuffmanTree #17949

mvdan opened this issue Nov 16, 2016 · 5 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mvdan
Copy link
Member

mvdan commented Nov 16, 2016

I just found this code in compress/bzip2/huffman.go, line 110:

if length > pairs[i].length {
        // If the code length decreases we shift in order to
        // zero any bits beyond the end of the code.
        length >>= 32 - pairs[i].length
        length <<= 32 - pairs[i].length
        length = pairs[i].length
}

I fail to see what the two shifts accomplish. Whatever the expressions do, the value is stomped by the final assignment. I'm not sure what the expected logic is here, or whether this is causing a bug of some sort. But the code seems wrong to me.

@mvdan
Copy link
Member Author

mvdan commented Nov 16, 2016

CC @agl who it seems wrote the code.

@dsnet
Copy link
Member

dsnet commented Nov 16, 2016

I can take this. I want to re-write the bzip2 huffman decoder anyways. It's the same decoder as what's used in compress/flate, only that the the flate version is faster.

@dsnet dsnet self-assigned this Nov 16, 2016
@dsnet dsnet added this to the Go1.9Maybe milestone Nov 16, 2016
@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label May 24, 2017
@bradfitz
Copy link
Contributor

@dsnet, fix looks easy. Send for 1.9?

@gopherbot
Copy link

CL https://golang.org/cl/44091 mentions this issue.

@dsnet
Copy link
Member

dsnet commented May 24, 2017

Filing seperate issue for re-writing bzip2 huffman logic. #20485

@golang golang locked and limited conversation to collaborators May 24, 2018
@rsc rsc unassigned dsnet Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants