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

Minor changes to dbwrapper to simplify support for other databases #7927

Merged
merged 4 commits into from Apr 25, 2016

Conversation

Projects
None yet
7 participants
@laanwj
Member

laanwj commented Apr 23, 2016

During my experiment with using LMDB as database I came across these database-agnostic changes to dbwrapper which make it easier to support other databases, or are small common-sense (I think) cleanups at most:

  • Pass parent CDBWrapper into CDBBatch and CDBIterator instead of obfuscation key (in the case of lmdb the batch needs to know about the database, to create a write transaction). This also looks cleaner.
  • Incrase privacy of methods that are implementation specific by moving them to a namespace: HandleError, GetObfuscateKey.
  • Make constructor of CDBIterator private - they rely on implementation details, others have no business instantiating these.
  • Remove GetObfuscateKeyHex - It is an unnecessary method as it is used only two times and only internally, and the whole implementation is HexStr(obfuscate_key).
  • Remove throw keywords in function signatures - Using throw() specifications in function signatures is not only not required in C++, it is considered deprecated for various reasons. It is not implemented by any of the common C++ compilers. Their usage is also inconsistent with the rest of the source code.

laanwj added some commits Apr 20, 2016

dbwrapper: Remove throw keywords in function signatures
Using throw() specifications in function signatures is not only
not required in C++, it is considered deprecated for
[various reasons](https://stackoverflow.com/questions/1055387/throw-keyword-in-functions-signature).
It is not implemented by any of the common C++ compilers. The usage is
also inconsistent with the rest of the source code.
dbwrapper: Remove CDBWrapper::GetObfuscateKeyHex
It is an unnecessary method as it is used only two times
and only internally, and the whole implementation is
HexStr(obfuscate_key).
dbwrapper: Pass parent CDBWrapper into CDBBatch and CDBIterator
Pass parent wrapper directly instead of obfuscation key. This
makes it possible for other databases which re-use this code
to use other properties from the database.

Add a namespace dbwrapper_private for private functions to be used
only in dbwrapper.h/cpp and dbwrapper_tests.
dbwrapper: Move `HandleError` to `dbwrapper_private`
HandleError is implementation-specific.
@sipa

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa Apr 23, 2016

Member

utACK 869cf12

Member

sipa commented Apr 23, 2016

utACK 869cf12

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke
Member

MarcoFalke commented Apr 23, 2016

utACK 869cf12

@btcdrak

This comment has been minimized.

Show comment
Hide comment
@btcdrak

btcdrak Apr 23, 2016

Member

Tested ACK 869cf12

Member

btcdrak commented Apr 23, 2016

Tested ACK 869cf12

@paveljanik

This comment has been minimized.

Show comment
Hide comment
@paveljanik

paveljanik Apr 23, 2016

Contributor

ACK 869cf12

throw() is still used in two other files on master:

src/support/allocators/secure.h
src/support/allocators/zeroafterfree.h

but it is not for this PR.

Contributor

paveljanik commented Apr 23, 2016

ACK 869cf12

throw() is still used in two other files on master:

src/support/allocators/secure.h
src/support/allocators/zeroafterfree.h

but it is not for this PR.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli
Member

jonasschnelli commented Apr 23, 2016

utACK 869cf12

@jamesob

This comment has been minimized.

Show comment
Hide comment
@jamesob

jamesob Apr 23, 2016

Member

utACK

Member

jamesob commented Apr 23, 2016

utACK

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Apr 25, 2016

Member

throw() is still used in two other files on master:

src/support/allocators/secure.h
src/support/allocators/zeroafterfree.h

Thanks for mentioning, although I think these are of a different class. They use throw() in the more conventional way to signify that a method or function doesn't ever throw. These could be replaced by the noexcept keyword in c++11.

Member

laanwj commented Apr 25, 2016

throw() is still used in two other files on master:

src/support/allocators/secure.h
src/support/allocators/zeroafterfree.h

Thanks for mentioning, although I think these are of a different class. They use throw() in the more conventional way to signify that a method or function doesn't ever throw. These could be replaced by the noexcept keyword in c++11.

@laanwj laanwj merged commit 869cf12 into bitcoin:master Apr 25, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Apr 25, 2016

Merge #7927: Minor changes to dbwrapper to simplify support for other…
… databases

869cf12 dbwrapper: Move `HandleError` to `dbwrapper_private` (Wladimir J. van der Laan)
b69836d dbwrapper: Pass parent CDBWrapper into CDBBatch and CDBIterator (Wladimir J. van der Laan)
878bf48 dbwrapper: Remove CDBWrapper::GetObfuscateKeyHex (Wladimir J. van der Laan)
74f7b12 dbwrapper: Remove throw keywords in function signatures (Wladimir J. van der Laan)

@str4d str4d referenced this pull request Aug 1, 2017

Merged

Fixes for GCC 7 #2545

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge bitcoin#7927: Minor changes to dbwrapper to simplify support fo…
…r other databases

869cf12 dbwrapper: Move `HandleError` to `dbwrapper_private` (Wladimir J. van der Laan)
b69836d dbwrapper: Pass parent CDBWrapper into CDBBatch and CDBIterator (Wladimir J. van der Laan)
878bf48 dbwrapper: Remove CDBWrapper::GetObfuscateKeyHex (Wladimir J. van der Laan)
74f7b12 dbwrapper: Remove throw keywords in function signatures (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge bitcoin#7927: Minor changes to dbwrapper to simplify support fo…
…r other databases

869cf12 dbwrapper: Move `HandleError` to `dbwrapper_private` (Wladimir J. van der Laan)
b69836d dbwrapper: Pass parent CDBWrapper into CDBBatch and CDBIterator (Wladimir J. van der Laan)
878bf48 dbwrapper: Remove CDBWrapper::GetObfuscateKeyHex (Wladimir J. van der Laan)
74f7b12 dbwrapper: Remove throw keywords in function signatures (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Sep 27, 2017

Merge bitcoin#7927: Minor changes to dbwrapper to simplify support fo…
…r other databases

869cf12 dbwrapper: Move `HandleError` to `dbwrapper_private` (Wladimir J. van der Laan)
b69836d dbwrapper: Pass parent CDBWrapper into CDBBatch and CDBIterator (Wladimir J. van der Laan)
878bf48 dbwrapper: Remove CDBWrapper::GetObfuscateKeyHex (Wladimir J. van der Laan)
74f7b12 dbwrapper: Remove throw keywords in function signatures (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Oct 12, 2017

Merge bitcoin#7927: Minor changes to dbwrapper to simplify support fo…
…r other databases

869cf12 dbwrapper: Move `HandleError` to `dbwrapper_private` (Wladimir J. van der Laan)
b69836d dbwrapper: Pass parent CDBWrapper into CDBBatch and CDBIterator (Wladimir J. van der Laan)
878bf48 dbwrapper: Remove CDBWrapper::GetObfuscateKeyHex (Wladimir J. van der Laan)
74f7b12 dbwrapper: Remove throw keywords in function signatures (Wladimir J. van der Laan)

codablock added a commit to codablock/dash that referenced this pull request Oct 19, 2017

Merge bitcoin#7927: Minor changes to dbwrapper to simplify support fo…
…r other databases

869cf12 dbwrapper: Move `HandleError` to `dbwrapper_private` (Wladimir J. van der Laan)
b69836d dbwrapper: Pass parent CDBWrapper into CDBBatch and CDBIterator (Wladimir J. van der Laan)
878bf48 dbwrapper: Remove CDBWrapper::GetObfuscateKeyHex (Wladimir J. van der Laan)
74f7b12 dbwrapper: Remove throw keywords in function signatures (Wladimir J. van der Laan)

@dagurval dagurval referenced this pull request Nov 2, 2017

Merged

Minor changes to dbwrapper #276

UdjinM6 added a commit to UdjinM6/dash that referenced this pull request Nov 8, 2017

Merge bitcoin#7927: Minor changes to dbwrapper to simplify support fo…
…r other databases

869cf12 dbwrapper: Move `HandleError` to `dbwrapper_private` (Wladimir J. van der Laan)
b69836d dbwrapper: Pass parent CDBWrapper into CDBBatch and CDBIterator (Wladimir J. van der Laan)
878bf48 dbwrapper: Remove CDBWrapper::GetObfuscateKeyHex (Wladimir J. van der Laan)
74f7b12 dbwrapper: Remove throw keywords in function signatures (Wladimir J. van der Laan)

zkbot added a commit to zcash/zcash that referenced this pull request Jan 15, 2018

Auto merge of #2598 - str4d:2074-dbwrapper, r=<try>
Bitcoin 0.12+ dbwrapper improvements

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6650
  - Only refactor - excludes obfuscation
- bitcoin/bitcoin#6777
  - Excluding obfuscation-related changes
- bitcoin/bitcoin#6865
- bitcoin/bitcoin#6823
- bitcoin/bitcoin#6873
- bitcoin/bitcoin#7927
  - Excluding first commit (already included) and second commit (obfuscation-related)
- bitcoin/bitcoin#8467

Part of #2074.

litecoinz-project pushed a commit to litecoinz-project/litecoinz that referenced this pull request Mar 15, 2018

zkbot added a commit to zcash/zcash that referenced this pull request Apr 3, 2018

Auto merge of #2598 - str4d:2074-dbwrapper, r=str4d
Bitcoin 0.12+ dbwrapper improvements

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6650
  - Only refactor - excludes obfuscation
- bitcoin/bitcoin#6777
  - Excluding obfuscation-related changes
- bitcoin/bitcoin#6865
- bitcoin/bitcoin#6823
- bitcoin/bitcoin#6873
- bitcoin/bitcoin#7927
  - Excluding first commit (already included) and second commit (obfuscation-related)
- bitcoin/bitcoin#8467

Part of #2074.

zkbot added a commit to zcash/zcash that referenced this pull request Apr 3, 2018

Auto merge of #2598 - str4d:2074-dbwrapper, r=str4d
Bitcoin 0.12+ dbwrapper improvements

Cherry-picked from the following upstream PRs:

- bitcoin/bitcoin#6650
  - Only refactor - excludes obfuscation
- bitcoin/bitcoin#6777
  - Excluding obfuscation-related changes
- bitcoin/bitcoin#6865
- bitcoin/bitcoin#6823
- bitcoin/bitcoin#6873
- bitcoin/bitcoin#7927
  - Excluding first commit (already included) and second commit (obfuscation-related)
- bitcoin/bitcoin#8467

Part of #2074.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment