Page MenuHomePhabricator
Differential D16209

util: Replace non-threadsafe strerror
ClosedPublic
Actions

Authored by PiRK on May 23 2024, 10:48.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC66e9e23fe2f8: util: Replace non-threadsafe strerror
Summary

Some uses of non-threadsafe strerror have snuck into the code since
they were removed in #4152. Add a wrapper SysErrorString for
thread-safe strerror alternatives and replace all uses of strerror
with this.

Add strerror to the locale-dependence linter to catch its use.
Add an exemption for strerror.cpp (the only allowed use).

Also fix a bug in the regexp so that _r and _s variants are detected
again.

This is a partial backport of core#24933
https://github.com/bitcoin/bitcoin/pull/24933/commits/46971c6dbfbc39ebbc74ab1ed8c00edc12859373
https://github.com/bitcoin/bitcoin/pull/24933/commits/e3a06a3c6cbb288ac89a2725cf71ae8adaebf35c
Depends on D16208 and D16212

Test Plan

ninja all check-all

Check that the linter catches occurences of strerror

Diff Detail

Event Timeline

PiRK created this revision.May 23 2024, 10:48
Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 23 2024, 10:48
PiRK requested review of this revision.May 23 2024, 10:48
teamcity edited the summary of this revision. (Show Details)May 23 2024, 10:49
PiRK added a child revision: D16210: util: Refactor SysErrorString logic.May 23 2024, 10:49
Harbormaster completed remote builds in B29105: Diff 47923.May 23 2024, 11:18
Fabien requested changes to this revision.May 23 2024, 13:52
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/init.cpp
180 ↗(On Diff #47923)

remove

src/util/syserror.cpp
22 ↗(On Diff #47923)

We are not defining this, it the branch will never be true. This is something that should be fixed before this diff imo

This revision now requires changes to proceed.May 23 2024, 13:52
PiRK mentioned this in D16212: [cmake] define STRERROR_R_CHAR_P.May 23 2024, 14:53
PiRK updated this revision to Diff 47930.May 23 2024, 15:13
PiRK edited the summary of this revision. (Show Details)
PiRK added a parent revision: D16212: [cmake] define STRERROR_R_CHAR_P.

undo accidental comment change, rebase on D16212 (add as a dependency)

Harbormaster completed remote builds in B29112: Diff 47930.May 23 2024, 15:47
Fabien accepted this revision.May 23 2024, 17:10
This revision is now accepted and ready to land.May 23 2024, 17:10
Closed by commit rABC66e9e23fe2f8: util: Replace non-threadsafe strerror (authored by laanwj <126646+laanwj@users.noreply.github.com>, committed by PiRK). · Explain WhyMay 23 2024, 19:11
This revision was automatically updated to reflect the committed changes.
PiRK mentioned this in rABCb44d125a8154: [cmake] define STRERROR_R_CHAR_P.
PiRK added a commit: rABC66e9e23fe2f8: util: Replace non-threadsafe strerror.
PiRK mentioned this in D16330: [lint] fix a false positive locale dependence detection.Jun 13 2024, 09:16
PiRK mentioned this in rABC5aa6eda04813: [lint] fix a false positive locale dependence detection.Jun 13 2024, 16:21

Revision Contents
Changeset List

PathSize
arcanist/
linter/
6 lines
src/
2 lines
3 lines
3 lines
util/
3 lines
4 lines
22 lines
17 lines
33 lines

Diff 47930

View Options

arcanist/linter/LocaleDependenceLinter.php

Loading...
View Options

src/CMakeLists.txt

Loading...
View Options

src/bitcoind.cpp

Loading...
View Options

src/init.cpp

Loading...
View Options

src/util/batchpriority.cpp

Loading...
View Options

src/util/fs.cpp

Loading...
View Options

src/util/sock.cpp

Loading...
View Options

src/util/syserror.h

Loading...
View Options

src/util/syserror.cpp

Loading...