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.

Sign up for GitHub

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

Jump to bottom

[Util] Replace LogPrintf (but not LogPrint) macro with regular function #2547

Merged
merged 1 commit into from
Sep 12, 2021
Merged

[Util] Replace LogPrintf (but not LogPrint) macro with regular function #2547

merged 1 commit into from
Sep 12, 2021

Conversation

random-zebra
Copy link

This commit combines together bitcoin#14209 [MarcoFalke] and bitcoin#17218 [jkczyz]

  • The first one replaces LogPrintf and LogPrint macros with functions:

    It is not possible to run the full test suite when configured with --enable-lcov, since logging is disabled currently so that "unnecessary branches are not analyzed". (See c8914b9)

    Fix this instead by replacing the macros with functions.

  • The second one restores the LogPrint macro:

    Calling LogPrint with a category that is not enabled results in evaluating the remaining function arguments, which may be arbitrarily complex (and possibly expensive) expressions. Defining LogPrint as a macro prevents this unnecessary expression evaluation.

    This is a partial revert of 14209. The decision to revert is discussed in 16688, which adds verbose logging for validation event notification.

@random-zebra random-zebra added this to the 5.4.0 milestone Sep 10, 2021
@random-zebra random-zebra self-assigned this Sep 10, 2021
@random-zebra random-zebra mentioned this pull request Sep 10, 2021
Merged
1 task
furszy
furszy approved these changes Sep 10, 2021
View reviewed changes
Copy link

@furszy furszy left a comment

Choose a reason for hiding this comment

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

nice, was needing this in another branch :).
ACK 14f2239

Fuzzbawls
Fuzzbawls approved these changes Sep 12, 2021
View reviewed changes
Copy link
Collaborator

@Fuzzbawls Fuzzbawls left a comment

Choose a reason for hiding this comment

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

ACK 14f2239

@random-zebra random-zebra merged commit 1286d3e into PIVX-Project:master Sep 12, 2021
furszy added a commit that referenced this pull request Sep 23, 2021
@furszy
Merge #2548: [Util] Add validation interface logging
2f16162 
d6fe150 Add logging for CValidationInterface events (random-zebra)
d1036c3 Refactor FormatStateMessage for clarity (Jeffrey Czyz)
d19c880 Format CValidationState properly in all cases (Jeffrey Czyz)
888eeac [Build] add util/validation build unit (random-zebra)
dbbf00b Add VALIDATION to BCLog::LogFlags (Jeffrey Czyz)

Pull request description:

  Backports bitcoin#16688 [jkczyz], which should help us debug some intermittent failures on GA (e.g. for `feature_reindex` or `wallet_basic` tests).

  > Add logging of CValidationInterface callbacks using a new VALIDATION log flag (see 12994). A separate flag is desirable as the logging can be noisy and thus may need to be disabled without affecting other logging.
  >
  > This could help debug issues where there may be race conditions at play, such as 12978.

  Based on top of
  - [x] #2547

ACKs for top commit:
  furszy:
    utACK d6fe150 after squash.

Tree-SHA512: affde45bf630d851c4e61b687e3a25cf5d33ae420630bef7c3d42f4632975a8197dccffa748f5bdb4899288232e73ae6c1967cde3e36b765f0d7d20ffdc4501d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@furszy furszy furszy approved these changes

@Fuzzbawls Fuzzbawls Fuzzbawls approved these changes

Assignees

@random-zebra random-zebra

Labels
Refactoring Upstream Util / log / libs
Projects
None yet
Milestone
5.4.0
Development

Successfully merging this pull request may close these issues.
3 participants
@random-zebra @furszy @Fuzzbawls