Minimal code changes to allow msvc compilation

sipsorcery commented at 2:06 pm on October 19, 2017: member

This PR is (hopefully) a starting point for the small number of changes required to allow Bitcoin Core to compile with Microsoft’s C/C++ compiler.

Some of the changes can definitely be improved upon. In particular ssize_t has been defined multiple times as I’m not familiar enough with the design to know the best location.

The changes may also break the non-msvc build, although that risk is reduced by using pre-compiler directives where possible. Travis will provide the initial answer to that question.

Minimal code changes to allow msvc compilation. e026c21ee6

laanwj added the label Windows on Oct 19, 2017

in src/bench/base58.cpp:29 in e026c21ee6

21@@ -22,7 +22,11 @@ static void Base58Encode(benchmark::State& state)
22         }
23     };
24     while (state.KeepRunning()) {
25+#ifndef _MSC_VER
26         EncodeBase58(buff.begin(), buff.end());
27+#else
28+		EncodeBase58(buff.begin()._Ptr, buff.end()._Ptr);
29+#endif

laanwj commented at 2:38 pm on October 19, 2017:

This would be portable and avoid the ifdef:

0EncodeBase58(buff.data(), buff.data() + buff.size())

(another option would be to make buff a vector, and you could just do EncodeBase58(buff))

in src/leveldb/db/c.cc:5 in e026c21ee6

4@@ -5,7 +5,9 @@
5 #include "leveldb/c.h"

laanwj commented at 2:41 pm on October 19, 2017:

Changes to leveldb should go through upstream, either https://github.com/google/leveldb or if it’s a specific change for us https://github.com/bitcoin-core/leveldb

in src/random.h:15 in e026c21ee6

11@@ -12,6 +12,14 @@
12 
13 #include <stdint.h>
14 
15+#ifdef _MSC_VER

laanwj commented at 2:44 pm on October 19, 2017:

This should likely go into a compatiblity header? I wonder why the ssize_t in netbase.cpp doesn’t trip it up.

in src/support/cleanse.cpp:39 in e026c21ee6

35@@ -32,7 +36,8 @@ void memory_cleanse(void *ptr, size_t len)
36        might try to eliminate "superfluous" memsets. If there's an easy way to
37        detect memset_s, it would be better to use that. */
38 #if defined(_MSC_VER)
39-    __asm;
40+    //__asm;

laanwj commented at 2:46 pm on October 19, 2017:

Please just remove the old code, don’t comment it out..

jonasschnelli commented at 6:44 pm on October 19, 2017: contributor

Concept ACK

sipsorcery commented at 11:13 am on October 20, 2017: member

Closing this PR as the code changes either need some more work or need to go into the leveldb repo. In the interim anyone wanting to build with VS can apply the code changes manually to their own source tree.

sipsorcery closed this on Oct 20, 2017

laanwj commented at 11:22 am on October 20, 2017: member

Why close? Review comments aren’t there to demotivate you, but to help you along getting it merged (after some rework, sure).

sipsorcery commented at 11:26 am on October 20, 2017: member

@laanwj Don’t worry I’ve got thicker skin than that :).

Closed this PR because as you mentioned a 3rd of the changes need to go to the leveldb repo. Another couple need some more work as they broke the Linux build. I’ll resubmit when I’ve been able to sort those issues out.

MarcoFalke referenced this in commit afa9600020 on Aug 13, 2018

PastaPastaPasta referenced this in commit ffb6b7c210 on May 26, 2021

PastaPastaPasta referenced this in commit 0687c9a9f5 on Jun 27, 2021

PastaPastaPasta referenced this in commit af67ca3268 on Jun 28, 2021

PastaPastaPasta referenced this in commit f4d7177988 on Jun 29, 2021

PastaPastaPasta referenced this in commit 2618b52d3c on Jun 29, 2021

PastaPastaPasta referenced this in commit fd2b028aba on Jun 29, 2021

PastaPastaPasta referenced this in commit 07e1a914af on Jun 29, 2021

PastaPastaPasta referenced this in commit a9bf129d18 on Jun 29, 2021

linuxsh2 referenced this in commit 78e9c711a6 on Jul 29, 2021

linuxsh2 referenced this in commit 638ecb1b0e on Sep 16, 2021

DrahtBot locked this on Dec 16, 2021

Minimal code changes to allow msvc compilation #11528