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

ADPCM, Crypto and hash table revision. #15

Merged
merged 21 commits into from
Jun 9, 2017
Merged

Conversation

DrSuperGood
Copy link
Contributor

Minor revision to ADPCM code for maintainability. Fixed a possible type cast logic error. Mostly documentation for maintainability.

Rewrote the MPQ cryptographic code to be more maintainable. Broke hashing and encryption into separate classes. Removed a lot of magic numbers from the code. Added support for processing buffers containing partial data.

Also revised the hash table code. Added locale support (not exposed at top level API yet). Correctly process deleted buckets when looking for files. Added more methods to modify hash table for future use.

Block table has not been revised outside of the cryptographic calls. Revision of this will occur at a later time as it is heavily coupled with file IO at the moment so might require a large revision. In the mean time it is probably a good idea to merge in these changes.

Removed some magic numbers and changed naming.
This is part of a bigger revision to break MpqCrypto into separate more
maintainable components for hashing and encryption.
Added support for processing deleted entries. Added power of 2 size
check. Added locale support. Removed dependency on memory mapping. Added
support to delete entries. Moved the responsibility for encryption up a
level. Replaced a magic number encryption key with a more sensible
constant.
Fixed some programming oversights. Gave more sensible names. Added
comments.
End search when finding recommended locale, only continue if not
recommended locale.
All hashes and encryption is now done by the new security classes.
@coveralls
Copy link

Coverage Status

Coverage decreased (-62.2%) to 28.605% when pulling a311033 on DrSuperGood:master into 1f1b80c on inwc3:master.

@DrSuperGood
Copy link
Contributor Author

Judging by the coverage drop, it is possible this commit has broken something in such a way that the automated tests are not picking it up as a failure.

Bad file key generation logic. A warning. A possible type cast error and
trying to fix block table corruption (not yet fully fixed).
@Frotty
Copy link
Member

Frotty commented Jun 7, 2017

seems like it. Now testIncompressibleFile is failing. You can also run the tests on your pc btw.

Thanks for the nice additions, hope to merge soon :) !

Use actual block table allocated size instead of number of blocks (off
by 1 error?). Remove old hash table writing code.
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.2%) to 89.667% when pulling 1c318e0 on DrSuperGood:master into 1f1b80c on inwc3:master.

@DrSuperGood
Copy link
Contributor Author

These coverage checks are strict!

I will write a test method for the hash table which will cover the currently unused functionality.

@Frotty
Copy link
Member

Frotty commented Jun 8, 2017

I don't think losing 1% is crucial, but yeah, it would be nice if you could add mpq examples that cover those cases (deleted bucket and different locale i believe?)

Also added useful hash table method.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.3%) to 90.561% when pulling eae3659 on DrSuperGood:master into 1f1b80c on inwc3:master.

@DrSuperGood
Copy link
Contributor Author

I give up trying to raise the coverage any more than that.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 90.716% when pulling d5cbd9c on DrSuperGood:master into 1f1b80c on inwc3:master.

@Frotty
Copy link
Member

Frotty commented Jun 9, 2017

This is ready to merge then I assume?

@DrSuperGood
Copy link
Contributor Author

DrSuperGood commented Jun 9, 2017

This is ready to merge then I assume?

As read as it ever will be. I am not getting that extra 0.1% coverage.

@Frotty Frotty merged commit d1a3a4d into inwc3:master Jun 9, 2017
@Frotty
Copy link
Member

Frotty commented Jun 9, 2017

Alright, thanks a lot again 👍

The changes are included in release 1.5.0

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

3 participants