validation: Document where we are intentionally ignoring bool return values from validation related functions#13864
Conversation
|
Should we not update the callers to react and log warnings on failures? E.g. this seems like a meaningful case: if (!file) {
LogPrintf("Warning: Could not open blocks file %s\n", path.string());
} else {
LogPrintf("Importing blocks file %s...\n", path.string());
if (!LoadExternalBlockFile(chainparams, file)) {
LogPrintf("Warning: No blocks loaded from %s\n", path.string());
}
} |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
637e735 to
72fbc1a
Compare
|
@Empact Increased logging should perhaps be a subject for another PR. Personally I’m not convinced that these cases are worth logging for – nobody seems to have been missing these log messages so far and we should be careful to decrease the signal to noise in our logging. It is already very verbose :-) |
|
Rebased! Please review :-) |
72fbc1a to
e551e4d
Compare
e551e4d to
339e8b6
Compare
|
Updated |
339e8b6 to
2fd3c6a
Compare
|
Rebased! :-) |
2fd3c6a to
383eb34
Compare
383eb34 to
81a168c
Compare
|
Rebased! :-) |
…ol returning functions from validation.{cpp,h} that modify state
|
Rebased! :-) |
81a168c to
71e5ce5
Compare
boolreturning functions fromvalidation.{cpp,h}that modify stateboolreturn value should not be discarded in the general case withNODISCARD(expands to[[nodiscard]]or__attribute__((warn_unused_result))depending on availability)This will help us identify code where we're we incorrectly assume that a certain function always succeeds in performing its state modification.