This makes it easier for us to replace it if desired, since it's now only in one spot. Also, it avoids the openssl include from allocators.h, which essentially forced openssl to be included from every compilation unit.
Also a few minor header cleanups.
I'm not a big fan of putting a single function in this object, but we use it at such low levels (allocators.h) that i don't want to stick it somewhere that would pull in other headers. Suggestions welcome.
Concept ACK.
I'm not against one function per object if that makes sense but let's not put all of these at the top level. For organizational purposes could put it in a subdirectory below src, e.g. support or util.
Sorry for the lack of response. I've been putting this off because this is part of a larger reorganization, and I figured I'd wait until it took shape before moving it somewhere since I'm not sure how it will end up looking. Right now, it'd make sense to move this along with the string/time/etc utils to some utils/ dir, but I'd also like to keep the consensus/app-specific stuff segregated in some way.
@laanwj After looking working on a refactor including this for a while, I still don't really have a good place for it. How about leaving it at the top for now and moving it to support/util when there's more stuff that can go there? For ex, we may consider moving random.o and other remaining openssl users into there, so that we have a more straightforward path to removal.
Otherwise, I'm happy to move it to whereever you prefer.
Well we are in the process of moving source files from the top level, so I'm not that happy to create more there. As for "let's move it later". If we know we are going to move it I prefer to create it in the right place (this avoids extra rebases as well as diff noise). I don't mind if this is the only file in support/ for now.
Ok, will do.
moved and rebased.
6 | +#include "cleanse.h" 7 | +#include <openssl/crypto.h> // for OPENSSL_cleanse() 8 | + 9 | +void memory_cleanse(void *ptr, size_t len) 10 | +{ 11 | + OPENSSL_cleanse(ptr, len);
4 spaces indentation, pretty please?
Grr.. still haven't managed to completely break this habit. Will fix.
utACK
0 | @@ -0,0 +1,12 @@ 1 | +// Copyright (c) 2009-2010 Satoshi Nakamoto 2 | +// Copyright (c) 2009-2015 The Bitcoin Core developers 3 | +// Distributed under the MIT software license, see the accompanying 4 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php. 5 | + 6 | +#include "cleanse.h" 7 | +#include <openssl/crypto.h> // for OPENSSL_cleanse()
Nit: Can you add a new-line here please?
0 | @@ -0,0 +1,12 @@ 1 | +// Copyright (c) 2009-2010 Satoshi Nakamoto 2 | +// Copyright (c) 2009-2015 The Bitcoin Core developers 3 | +// Distributed under the MIT software license, see the accompanying 4 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php. 5 | + 6 | +#ifndef BITCOIN_CLEANSE_H 7 | +#define BITCOIN_CLEANSE_H 8 | +#include <stdlib.h>
Nit: Same here, new-line is missing.
0 | @@ -0,0 +1,12 @@ 1 | +// Copyright (c) 2009-2010 Satoshi Nakamoto 2 | +// Copyright (c) 2009-2015 The Bitcoin Core developers 3 | +// Distributed under the MIT software license, see the accompanying 4 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php. 5 | + 6 | +#ifndef BITCOIN_CLEANSE_H
This should be BITCOIN_SUPPORT_CLEANSE_H and you are currently missing the header end comment :).
utACK after fixing the comments.
Nits addressed.
This makes it easier for us to replace it if desired, since it's now only in
one spot. Also, it avoids the openssl include from allocators.h, which
essentially forced openssl to be included from every compilation unit.
Thanks, utACK!
utACK
utACK