Skip to content

Conversation

Empact
Copy link
Contributor

@Empact Empact commented Jan 6, 2019

This corrects all violations of -Wzero-as-null-pointer-constant identified in the Qt codebase.

These changes are extracted from #15112 as suggested by @MarcoFalke to ease review. This is in service of enabling -Wzero-as-null-pointer-constant, which should eliminate this as a concern going forward.

Note there are 2 non-Qt changes: src/test/allocator_tests.cpp and src/wallet/db.cpp.

@Empact Empact force-pushed the qt-zero-as-null-pointer-constant branch from 1667746 to 60a2a0b Compare January 6, 2019 10:35
@@ -338,7 +338,7 @@ bool BerkeleyBatch::VerifyEnvironment(const fs::path& file_path, std::string& er
BerkeleyEnvironment* env = GetWalletEnv(file_path, walletFile);
fs::path walletDir = env->Directory();

LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0));
LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(nullptr, nullptr, nullptr));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hebasto
Copy link
Member

hebasto commented Jan 6, 2019

Concept ACK.

@@ -300,8 +300,8 @@ QVariant AddressTableModel::headerData(int section, Qt::Orientation orientation,

Qt::ItemFlags AddressTableModel::flags(const QModelIndex &index) const
{
if(!index.isValid())
return 0;
if (!index.isValid()) return Qt::NoItemFlags;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -45,7 +45,7 @@ class ShutdownWindow : public QWidget
Q_OBJECT

public:
explicit ShutdownWindow(QWidget *parent=0, Qt::WindowFlags f=0);
explicit ShutdownWindow(QWidget *parent=nullptr, Qt::WindowFlags f=Qt::Widget);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@luke-jr
Copy link
Member

luke-jr commented Jan 6, 2019

Rationale? Changing code style stuff for no reason isn't useful...

@practicalswift
Copy link
Contributor

Concept ACK

@practicalswift
Copy link
Contributor

practicalswift commented Jan 6, 2019

@luke-jr The change @Empact is suggesting is not a code style change.

Reasons to use nullptr include:

  • nullptr has a well-specified (very restrictive) type, and thus works in more scenarios where type deduction might do the wrong thing on NULL or 0.
  • Improves readability.
  • Minimises surprises: nullptr cannot be confused with an int.

Enabling -Wzero-as-null-pointer-constant will help developers follow our developer guide which is a good thing. This in turn means that human reviewers will not have to point out the benefits of nullptr ever again :-)

@practicalswift
Copy link
Contributor

utACK 60a2a0b24275757dee767deafadd86d24889bdf8

@hebasto
Copy link
Member

hebasto commented Jan 6, 2019

utACK 60a2a0b24275757dee767deafadd86d24889bdf8.
nit: could clang-format-diff.py be applied?

@promag
Copy link
Contributor

promag commented Jan 6, 2019

Concept ACK, agree it improves code.

I don't think this should be merged without something to prevent more zero-as-null-pointer.

@practicalswift
Copy link
Contributor

@promag That is taken care of in #15112, right? :-)

@laanwj
Copy link
Member

laanwj commented Jan 7, 2019

I don't think this should be merged without something to prevent more zero-as-null-pointer.

I agree, as discussed many times, we don't want PR after PR fixing 'straggler' occurrences. It's not that important that it warrants some kind of crusade.

But if #15112 enables failing the build on this I'm okay with that and they should probably be merged together.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jan 9, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

  • #15157 (rpc: Bumpfee units change, satoshis to BTC by benthecarman)
  • #15153 (gui: Add Open Wallet menu by promag)
  • #15101 (gui: Add WalletController by promag)
  • #15040 (qt: Add workaround for QProgressDialog bug on macOS by hebasto)
  • #13880 (gui: Try shutdown also on failure by MarcoFalke)
  • #12096 ([rpc] [wallet] Allow specifying the output index when using bumpfee by kallewoof)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

practicalswift and others added 2 commits January 13, 2019 03:25
Also used type-appropriate enum values such as Qt::NoItemFlags in
some cases.

All cases identified via -Wzero-as-null-pointer-constant
@Empact Empact force-pushed the qt-zero-as-null-pointer-constant branch from 60a2a0b to 3a0e76f Compare January 13, 2019 17:10
@Empact
Copy link
Contributor Author

Empact commented Jan 13, 2019

Rebased for #13216

@practicalswift
Copy link
Contributor

utACK 3a0e76f

1 similar comment
@laanwj
Copy link
Member

laanwj commented Jan 14, 2019

utACK 3a0e76f

benthecarman pushed a commit to benthecarman/bitcoin that referenced this pull request Jan 14, 2019
3a0e76f Replace remaining 0 with nullptr in Qt code (Ben Woosley)
9096276 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift)

Pull request description:

  This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase.

  These changes are extracted from bitcoin#15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward.

  Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`.

Tree-SHA512: 206bd668802147ba42bc413c2d7d259cb59aca9ec1da74a6bf2ca3932e60ae492faacbc61bcee0fd6b4b49a4d59d075b7e5404f0526b36c47718f9b0587e7768
@laanwj laanwj merged commit 3a0e76f into bitcoin:master Jan 14, 2019
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Feb 27, 2020
Summary:
3a0e76fc12b91b2846d756981e15f09b767a9c37 Replace remaining 0 with nullptr in Qt code (Ben Woosley)
9096276e0b2d5b7e19af9a5f3c144ef108ee55e0 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift)

---

Pull request description:

This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase.

These changes are extracted from #15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward.

Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`.

---

This is a backport from Core [[bitcoin/bitcoin#15114 | PR15114]] (bitcoin/bitcoin#15114)

Test Plan:
   ninja check
   ninja check-functional

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5341
ftrader pushed a commit to bitcoin-cash-node/bitcoin-cash-node that referenced this pull request Apr 16, 2020
Summary:
3a0e76fc12b91b2846d756981e15f09b767a9c37 Replace remaining 0 with nullptr in Qt code (Ben Woosley)
9096276e0b2d5b7e19af9a5f3c144ef108ee55e0 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift)

---

Pull request description:

This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase.

These changes are extracted from #15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward.

Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`.

---

This is a backport from Core [[bitcoin/bitcoin#15114 | PR15114]] (bitcoin/bitcoin#15114)

Test Plan:
   ninja check
   ninja check-functional

Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix

Differential Revision: https://reviews.bitcoinabc.org/D5341
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 21, 2021
3a0e76f Replace remaining 0 with nullptr in Qt code (Ben Woosley)
9096276 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift)

Pull request description:

  This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase.

  These changes are extracted from bitcoin#15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward.

  Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`.

Tree-SHA512: 206bd668802147ba42bc413c2d7d259cb59aca9ec1da74a6bf2ca3932e60ae492faacbc61bcee0fd6b4b49a4d59d075b7e5404f0526b36c47718f9b0587e7768
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 23, 2021
3a0e76f Replace remaining 0 with nullptr in Qt code (Ben Woosley)
9096276 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift)

Pull request description:

  This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase.

  These changes are extracted from bitcoin#15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward.

  Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`.

Tree-SHA512: 206bd668802147ba42bc413c2d7d259cb59aca9ec1da74a6bf2ca3932e60ae492faacbc61bcee0fd6b4b49a4d59d075b7e5404f0526b36c47718f9b0587e7768
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 24, 2021
3a0e76f Replace remaining 0 with nullptr in Qt code (Ben Woosley)
9096276 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift)

Pull request description:

  This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase.

  These changes are extracted from bitcoin#15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward.

  Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`.

Tree-SHA512: 206bd668802147ba42bc413c2d7d259cb59aca9ec1da74a6bf2ca3932e60ae492faacbc61bcee0fd6b4b49a4d59d075b7e5404f0526b36c47718f9b0587e7768
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 24, 2021
3a0e76f Replace remaining 0 with nullptr in Qt code (Ben Woosley)
9096276 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift)

Pull request description:

  This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase.

  These changes are extracted from bitcoin#15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward.

  Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`.

Tree-SHA512: 206bd668802147ba42bc413c2d7d259cb59aca9ec1da74a6bf2ca3932e60ae492faacbc61bcee0fd6b4b49a4d59d075b7e5404f0526b36c47718f9b0587e7768
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 24, 2021
3a0e76f Replace remaining 0 with nullptr in Qt code (Ben Woosley)
9096276 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift)

Pull request description:

  This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase.

  These changes are extracted from bitcoin#15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward.

  Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`.

Tree-SHA512: 206bd668802147ba42bc413c2d7d259cb59aca9ec1da74a6bf2ca3932e60ae492faacbc61bcee0fd6b4b49a4d59d075b7e5404f0526b36c47718f9b0587e7768
UdjinM6 pushed a commit to UdjinM6/dash that referenced this pull request Aug 24, 2021
3a0e76f Replace remaining 0 with nullptr in Qt code (Ben Woosley)
9096276 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift)

Pull request description:

  This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase.

  These changes are extracted from bitcoin#15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward.

  Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`.

Tree-SHA512: 206bd668802147ba42bc413c2d7d259cb59aca9ec1da74a6bf2ca3932e60ae492faacbc61bcee0fd6b4b49a4d59d075b7e5404f0526b36c47718f9b0587e7768
Munkybooty pushed a commit to Munkybooty/dash that referenced this pull request Aug 24, 2021
3a0e76f Replace remaining 0 with nullptr in Qt code (Ben Woosley)
9096276 Don't use zero as null pointer constant (-Wzero-as-null-pointer-constant) (practicalswift)

Pull request description:

  This corrects all violations of `-Wzero-as-null-pointer-constant` identified in the Qt codebase.

  These changes are extracted from bitcoin#15112 as suggested by @MarcoFalke to ease review. This is in service of enabling `-Wzero-as-null-pointer-constant`, which should eliminate this as a concern going forward.

  Note there are 2 non-Qt changes: `src/test/allocator_tests.cpp` and `src/wallet/db.cpp`.

Tree-SHA512: 206bd668802147ba42bc413c2d7d259cb59aca9ec1da74a6bf2ca3932e60ae492faacbc61bcee0fd6b4b49a4d59d075b7e5404f0526b36c47718f9b0587e7768
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
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.

8 participants