Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented May 29, 2020

once_flag is a helper (as the name might suggest) to execute a callable only once. Thus, the scope of the flag does never need to extend beyond where the callable is called. Typically this is function scope.

Move all the flags to function scope to

  • simplify code review
  • avoid mistakes where similarly named flags are accidentally exchanged
  • avoid polluting the global scope

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Concept ACK.
Is g_ prefix in variable names still relevant now?

@maflcko
Copy link
Member Author

maflcko commented May 30, 2020

Is g_ prefix in variable names still relevant now?

Yes, they still have static storage duration, also the diff is smaller keeping it. Finally, to keep code consistent with other instances like g_logger.

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK fa9c675, tested on Linux Mint 19.3 (x86_64).

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

Code review ACK fa9c675.

@maflcko maflcko merged commit 1f7fe59 into bitcoin:master Jun 2, 2020
@maflcko maflcko deleted the 2005-NoGlobalOnce branch June 2, 2020 11:07
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 2, 2020
fa9c675 Limit scope of all global std::once_flag (MarcoFalke)

Pull request description:

  `once_flag` is  a helper (as the name might suggest) to execute a callable only once. Thus, the scope of the flag does never need to extend beyond where the callable is called. Typically this is function scope.

  Move all the flags to function scope to
  * simplify code review
  * avoid mistakes where similarly named flags are accidentally exchanged
  * avoid polluting the global scope

ACKs for top commit:
  hebasto:
    ACK fa9c675, tested on Linux Mint 19.3 (x86_64).
  promag:
    Code review ACK fa9c675.

Tree-SHA512: 095a0c11d93d0ddcb82b3c71676090ecc7e3de3d5e7a2a63ab2583093be279242acac43523bbae2060b4dcfa8f92b54256a0e91fbbae78fa92d2d49e9db62e57
zkbot added a commit to zcash/zcash that referenced this pull request Sep 29, 2020
Locked memory manager

Add a pool for locked memory chunks, replacing `LockedPageManager`.

Cherry-picked from the following upstream PRs:
- bitcoin/bitcoin#8321
- bitcoin/bitcoin#8753
- bitcoin/bitcoin#9063
- bitcoin/bitcoin#9070
- bitcoin/bitcoin#11385
- bitcoin/bitcoin#12048
  - Excludes change to benchmark.
- bitcoin/bitcoin#15117
- bitcoin/bitcoin#16161
  - Excludes Travis CI changes.
  - Includes change from bitcoin/bitcoin#13163
- bitcoin/bitcoin#15600
- bitcoin/bitcoin#18443
- Assorted small changes from:
  - bitcoin/bitcoin#9233
  - bitcoin/bitcoin#10483
  - bitcoin/bitcoin#10645
  - bitcoin/bitcoin#10969
  - bitcoin/bitcoin#11351
- bitcoin/bitcoin#19111
  - Excludes change to `src/rpc/server.cpp`
- bitcoin/bitcoin#9804
  - Only the commit for `src/key.cpp`
- bitcoin/bitcoin#9598
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Feb 24, 2021
Summary:
> `once_flag` is a helper (as the name might suggest) to execute a callable only once. Thus, the scope of the flag does never need to extend beyond where the callable is called. Typically this is function scope.
>
> Move all the flags to function scope to
>
>     simplify code review
>     avoid mistakes where similarly named flags are accidentally exchanged
>     avoid polluting the global scope
>

This is a backport of [[bitcoin/bitcoin#19111 | core#19111]]

Depends on D9260

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D9264
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants