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

Enable GCC/clang -Wshadow compiler flag #136

Merged
merged 1 commit into from
Nov 16, 2016
Merged

Enable GCC/clang -Wshadow compiler flag #136

merged 1 commit into from
Nov 16, 2016

Conversation

droark
Copy link

@droark droark commented Nov 11, 2016

GCC/clang will now throw warnings whenever variables shadow other variables. Armory code was cleaned up to remove these warnings. (Third party code - LMDB and Crypto++ - and code generated by SWIG was not cleaned up.)

@droark
Copy link
Author

droark commented Nov 11, 2016

Ran CppBlockUtilsTests and DB1kIter. No new tests failed. (Three fail under OS X when running CppBlockUtilsTests.) It's a good idea to double check the code changes, though. I mostly added underscores but there were two variables that were duplicates of the variables they shadowed, with no apparent need for it. I deleted those shadow variables.

@goatpig
Copy link
Owner

goatpig commented Nov 11, 2016

I have a big commit coming with p2sh + p2wsh wallets with signing, as well as compressed keys. Could you rebase on that after I push that code? It's for tomorrow or the day after I believe. Code is mostly done, writing unit tests and debugging as we speak.

@droark
Copy link
Author

droark commented Nov 11, 2016

Gaaaaah. Wish I had known this earlier. :/ I'll rebase once it's pushed.

@goatpig
Copy link
Owner

goatpig commented Nov 11, 2016

Yeah sorry about that, I tend to do few massive pushes lately.

@goatpig
Copy link
Owner

goatpig commented Nov 14, 2016

The big commit is pushed, go ahead

@droark droark force-pushed the dev branch 2 times, most recently from dc3e14a to 728a8d2 Compare November 14, 2016 23:14
@droark
Copy link
Author

droark commented Nov 14, 2016

Rebased and slightly revised. The biggest change is that I went ahead and fixed the warnings in the the LMDB code. This should be fine. However, all these changes need to be carefully considered before accepting them.

GCC/clang will now throw warnings whenever variables shadow other variables. Armory code was cleaned up to remove these warnings. (Third party code - LMDB and Crypto++ - and code generated by SWIG was not cleaned up.)
@goatpig
Copy link
Owner

goatpig commented Nov 15, 2016

The changes to the custom lmdb cpp headers are fine, but I can't take any changes to mdb.c. It's basically pointless, as whenever I update LMDB, the entire changeset goes out the toilet.

@goatpig goatpig merged commit 7bad6a5 into goatpig:dev Nov 16, 2016
droark added a commit to droark/BitcoinArmory that referenced this pull request Jul 12, 2018
1a33fa2 Merge goatpig#138: Expose ripemd160
15ac927 Expose ripemd160
8863aa7 Merge goatpig#136: Extend btc_script_classify to correctly cover P2SH, P2PK
2540312 Extend btc_script_classify to correctly cover P2SH, P2PK
07a21a0 Merge goatpig#134: Add some script size sanity checks
269ab0e Add some script size sanity checks

git-subtree-dir: cppForSwig/libbtc
git-subtree-split: 1a33fa20cdd9d9ae19588ce23453bbbe864355b2
droark added a commit to droark/BitcoinArmory that referenced this pull request Jul 16, 2018
1a33fa20 Merge goatpig#138: Expose ripemd160
15ac927f Expose ripemd160
8863aa77 Merge goatpig#136: Extend btc_script_classify to correctly cover P2SH, P2PK
25403128 Extend btc_script_classify to correctly cover P2SH, P2PK
07a21a0a Merge goatpig#134: Add some script size sanity checks
269ab0e2 Add some script size sanity checks

git-subtree-dir: cppForSwig/libbtc
git-subtree-split: 1a33fa20cdd9d9ae19588ce23453bbbe864355b2
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