Prefer direct access to cached pathDataDir, rather than calling a function #1281

pull jgarzik wants to merge 1 commits into bitcoin:master from jgarzik:datadir changing 9 files +24 −18

jgarzik commented at 7:13 AM on May 13, 2012: contributor

Even with the GetDataDir() cached system, a function call is overkill for the vast majority of uses in the code. The values are static once the program is initialized, so initialize pathDataDir once.

GetDataDir() is renamed to GetPrepDataDir() to better indicate that it has side effects.

Curious to see whether people think this is worth merging or not. I think it cleans up the code, and is ever so slightly more CPU efficient for most use cases.
Prefer direct access to cached pathDataDir, rather than calling a function 44095c1fa0
laanwj commented at 8:18 AM on May 13, 2012: member

Hmm.. on one hand, I agree that it doesn't strictly need to be a function, on the other hand, I don't really like exported free-for-all global state as you cannot enforce read-only. It's almost a necessity to have getters/setters in C++ for this reason.

I don't buy "CPU efficient" here. DataDir is used sparsely with I/O, not with intensive computation. The decision should completely be based on "what results in readable and maintainable code".

in src/util.cpp:None in 44095c1fa0

 859 | @@ -859,6 +860,9 @@ void PrintExceptionContinue(std::exception* pex, const char* pszThread)
 860 |  
 861 |      fs::create_directory(path);
 862 |  
 863 | +    if (!fNetSpecific)
 864 | +        pathDataDir = path;
 865 | +

sipa commented at 9:41 AM on May 13, 2012:

I think you want to put the net-specific path in pathDataDir - that's the one used almost everywhere but in GetConfigFile.

sipa commented at 9:42 AM on May 13, 2012: member

@laanwj In all but the first call, it doesn't even need a mutex - it just returns the value cached in pathCached[].
Diapolo commented at 9:46 AM on May 13, 2012: none

Wouldn't it be nicer to simply make the GetDataDir() function use a static constant but non global variable? I don't really like the solution this pull offers.
jgarzik commented at 6:46 PM on May 13, 2012: contributor

@sipa: You're right. That's a bug... I meant to use the net-specific path.
jgarzik commented at 7:21 PM on May 14, 2012: contributor

closing due to disinterest
jgarzik closed this on May 14, 2012
jgarzik deleted the branch on Aug 24, 2014
suprnurd referenced this in commit 534effaf85 on Dec 5, 2017
lateminer referenced this in commit 486d4f8661 on Jan 22, 2019
lateminer referenced this in commit a293072cdb on May 6, 2020
DrahtBot locked this on Sep 8, 2021

Contributors