MOVEONLY-ISH: allocators: split allocators and pagelocker #5810

pull theuni wants to merge 1 commits into bitcoin:master from theuni:allocators changing 12 files +129 −102

theuni commented at 7:55 PM on February 20, 2015: member

Part of a larger effort to clean up dependencies, this is a straightforward change. Headers were fixed up as necessary, and I added the bitcoin-config.h include in pagelocker.cpp in case it was picked up from elsewhere before.

Pagelocker is only used for sensitive info, so don't require it for things that just want zero-after-free. In particular, this un-tethers streams from boost and global app-state.
paveljanik commented at 8:41 PM on February 20, 2015: contributor

utACK, I like this style of cleanup.
theuni force-pushed on Feb 20, 2015
theuni commented at 8:53 PM on February 20, 2015: member

nits addressed.
jtimon commented at 11:58 PM on February 20, 2015: contributor

ut ACK
laanwj commented at 9:47 AM on February 23, 2015: member

Agree on the idea, although I'm wondering whether src/allocators should be a first-class directory or this stuff should be in the more general src/support.
laanwj added the label Improvement on Feb 23, 2015
theuni commented at 7:31 PM on February 23, 2015: member

@laanwj To clarify, what do you intend src/support to house?

Either way, I'm happy to move it if you'd prefer it there.
jtimon commented at 8:14 PM on February 23, 2015: contributor

Maybe we can move some utils there or maybe we will find out what else to move there later. Just thinking out loud...
laanwj commented at 12:26 PM on February 25, 2015: member
@theuni Let me try: src/support is for OS support that means lower-level utility functions that depend on calls to the OS, with potential side effects.
- Currently cleanse is in there, which makes sense.
- Allocators also fall squarely into that category (also isn't cleanse mostly/only used by the allocators?).
- Low-level synchronization primitives would also fit here.
- Pure utility functions (e.g. utilstrencodings) would not belong there. They are OS-independent and should even work in a completely static computational environment like moxiebox.
This is analogous to leveldb's port_* and env_*. There is a slight overlap with compat, although compat deals with (pure) functions that lack from a certain C/C++ library but are part of some standard.

Maybe we should write a plan how we like the tree organized so that these things are more clear. Maybe there are good arguments why allocators makes sense as a category/library of itself, I just think that the default should be to try to fit something into a broad category and not create too many subdirs of src/.
theuni commented at 7:09 PM on February 25, 2015: member

@laanwj Ok, that sounds good. To me, 'support' implied that the functionality within would be provided by 3rd party libs. Your description makes sense and I follow now, thanks.

If there were more allocators, or more planned, I'd argue that they make sense by themselves. But 2 really isn't enough to justify that.

I'll move and rename to fit.
theuni force-pushed on Feb 25, 2015

allocators: split allocators and pagelocker

Pagelocker is only needed for secure (usually wallet) operations, so don't make
the zero-after-free allocator depend on it.

cc33702684

theuni commented at 9:13 PM on February 26, 2015: member

Moved to support.
jtimon commented at 11:17 PM on February 26, 2015: contributor

re-utACK
sipa commented at 11:03 AM on March 1, 2015: member

utACK
jgarzik commented at 2:00 PM on March 11, 2015: contributor

ut ACK
sipa commented at 1:45 PM on March 12, 2015: member

Needs rebase.
laanwj referenced this in commit 3811a5025e on Mar 20, 2015
laanwj commented at 11:33 AM on March 20, 2015: member

Merged via d7d187e8a451ae946fa14cead7962edbe0046f12 (only conflict was a #include in the tests)
laanwj closed this on Mar 20, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels