Page MenuHomePhabricator

Don't assert(...) with side effects

Authored by Fabien on Jun 10 2019, 13:09.



This diffs fixes the assertions introduced in D3232 and adds a linter to
avoid reproducing the issue.

The fix is a backport of core PR14088
The linter is based on the second commit, but integrated into arcanist.

Test Plan
./src/test/test_bitcoin -t scheduler_tests

Revert the changes in scheduler_tests and run:

arc lint -- src/test/scheduler_tests.cpp

The linter should catch the errors

Diff Detail

rABC Bitcoin ABC
Lint OK
No Unit Test Coverage
Build Status
Buildable 6248
Build 10543: Bitcoin ABC Buildbot (legacy)
Build 10542: arc lint + arc unit

Event Timeline

Fabien created this revision.Jun 10 2019, 13:09
Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 10 2019, 13:09
jasonbcox accepted this revision.Jun 10 2019, 16:43
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.

Nit: It may improve this message to refer to what the original comment did in test/lint/

# PRE31-C (SEI CERT C Coding Standard):
# "Assertions should not contain assignments, increment, or decrement operators."
This revision is now accepted and ready to land.Jun 10 2019, 16:43
Fabien updated this revision to Diff 9294.Jun 10 2019, 20:18

Rebase and update error message.

This revision was automatically updated to reflect the committed changes.