Skip to content

Commit 856c887

Browse files
vasildMarcoFalke
authored andcommitted
ArgsManager: return path by value from GetBlocksDirPath()
`ArgsManager::m_cached_blocks_path` is protected by `ArgsManager::cs_args` and returning a reference to it after releasing the mutex is unsafe. To resolve this, return a copy of the path. This has some performance penalty which is presumably ok, given that paths are a few 100s bytes at most and `GetBlocksDirPath()` is not called often. This silences the following (clang 18): ``` common/args.cpp:288:31: error: returning variable 'm_cached_blocks_path' by reference requires holding mutex 'cs_args' [-Werror,-Wthread-safety-reference-return] 288 | if (!path.empty()) return path; | ^ ``` Do the same with `ArgsManager::GetDataDir()`, `ArgsManager::GetDataDirBase()` and `ArgsManager::GetDataDirNet()`.
1 parent fa3d930 commit 856c887

File tree

2 files changed

+6
-6
lines changed

2 files changed

+6
-6
lines changed

src/common/args.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ fs::path ArgsManager::GetPathArg(std::string arg, const fs::path& default_value)
277277
return result.has_filename() ? result : result.parent_path();
278278
}
279279

280-
const fs::path& ArgsManager::GetBlocksDirPath() const
280+
fs::path ArgsManager::GetBlocksDirPath() const
281281
{
282282
LOCK(cs_args);
283283
fs::path& path = m_cached_blocks_path;
@@ -302,7 +302,7 @@ const fs::path& ArgsManager::GetBlocksDirPath() const
302302
return path;
303303
}
304304

305-
const fs::path& ArgsManager::GetDataDir(bool net_specific) const
305+
fs::path ArgsManager::GetDataDir(bool net_specific) const
306306
{
307307
LOCK(cs_args);
308308
fs::path& path = net_specific ? m_cached_network_datadir_path : m_cached_datadir_path;

src/common/args.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,21 +215,21 @@ class ArgsManager
215215
*
216216
* @return Blocks path which is network specific
217217
*/
218-
const fs::path& GetBlocksDirPath() const;
218+
fs::path GetBlocksDirPath() const;
219219

220220
/**
221221
* Get data directory path
222222
*
223223
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
224224
*/
225-
const fs::path& GetDataDirBase() const { return GetDataDir(false); }
225+
fs::path GetDataDirBase() const { return GetDataDir(false); }
226226

227227
/**
228228
* Get data directory path with appended network identifier
229229
*
230230
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
231231
*/
232-
const fs::path& GetDataDirNet() const { return GetDataDir(true); }
232+
fs::path GetDataDirNet() const { return GetDataDir(true); }
233233

234234
/**
235235
* Clear cached directory paths
@@ -420,7 +420,7 @@ class ArgsManager
420420
* @param net_specific Append network identifier to the returned path
421421
* @return Absolute path on success, otherwise an empty path when a non-directory path would be returned
422422
*/
423-
const fs::path& GetDataDir(bool net_specific) const;
423+
fs::path GetDataDir(bool net_specific) const;
424424

425425
/**
426426
* Return -regtest/-signet/-testnet/-chain= setting as a ChainType enum if a

0 commit comments

Comments
 (0)