change "char pch[200000]" to "new char[200000]"#4236
change "char pch[200000]" to "new char[200000]"#4236arowser wants to merge 1 commit intobitcoin:masterfrom
Conversation
src/util.cpp
Outdated
There was a problem hiding this comment.
sizeof(pch) will be 4 or 8 here. You should use the correct amount (probably best to define a constant for the 200000).
There was a problem hiding this comment.
If we're going to use dynamic memory, please use a properly managed way
(e.g. use a vector).
There was a problem hiding this comment.
Switching to dynamic memory makes sense, 200KB is too much to allocate on the stack. It could exhaust the per-thread stack on some platforms.
Using a vector sounds good to me.
|
Indeed. An RAII vector is superior to something you must remember to manually delete[]. |
|
@arowser are you going to update this to use a vector instead? |
|
@laanwj Yes, I'll update it later, sorry for late. I'm have more than ten years experience in c lang, but I'm a c++ new guy, so I'm studying how change char * to vector in c++ and make sure the chang is ok before I push it again, thanks (: |
src/util.cpp
Outdated
There was a problem hiding this comment.
Use
std::vector<unsigned char>
instead.
|
Looks OK, but I'm a bit wary to merge as it affects entropy collection for randomness. I'd like someone with windows to verify that this does the right thing. |
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4236_ea7875e8c3da92b0cbddb9c794843cfcf6c0a80c/ for binaries and test log. |
|
Closing in favor of #4392 |
Change the big size local var "char pch[200000]" to "new char[200000]" to avoid the possibly stack overflow.