Skip to content

Read/write mempool.dat as a binary. #9813

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
laanwj merged 1 commit into from
Feb 21, 2017
Merged

Read/write mempool.dat as a binary. #9813

laanwj merged 1 commit into from
Feb 21, 2017

Conversation

@paveljanik
Copy link
Contributor

@paveljanik paveljanik commented Feb 20, 2017

mempool.dat is a binary file and thus it should be read/written as such.

Fixes #9810.

I can't test on Windows though.

@paveljanik
Read/write mempool.dat as a binary.
171fc91 
mempool.dat is a binary file and thus it should be read/written as such.

Fixes bitcoin#9810.
@laanwj
Copy link
Member

laanwj commented Feb 20, 2017 •
edited
Loading

Obviously correct ACK 171fc91

<paveljanik> binary on write?
<paveljanik> Windows...
<wumpus> that could be the explanation, good catch! how is the file opened?
<wumpus> if its opened in text (instead of binary) mode, windows will convert \n to \r\n
<paveljanik> it is written with "w" and opened with "r"
<wumpus> that should be "wb" and "rb"
<wumpus> so you've found the issue, congrats

I wonder if we have any more instances of this.

Edit: no, I've grepped for uses for r and rw and w on fopen, and the other cases all deal with text files (log file, pid file) or are only applicable to POSIX platforms (/dev/urandom in secp256k1 tests).

@laanwj laanwj added this to the 0.14.0 milestone Feb 20, 2017
@theuni
Copy link
Member

theuni commented Feb 20, 2017

Good catch. utACK 171fc91

@paveljanik paveljanik mentioned this pull request Feb 20, 2017
Closed
@achow101
Copy link
Member

achow101 commented Feb 20, 2017 •
edited
Loading

ACK 171fc91

Tested on windows 10 and ubuntu 16.04.


Edit: It works. It is overwriting the old mempool.dat properly.

gmaxwell
gmaxwell approved these changes Feb 21, 2017
View reviewed changes
Copy link
Contributor

@gmaxwell gmaxwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

@fanquake
Copy link
Member

fanquake commented Feb 21, 2017

utACK 171fc91

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Feb 21, 2017

utACK 171fc91

@laanwj laanwj merged commit 171fc91 into bitcoin:master Feb 21, 2017
laanwj added a commit that referenced this pull request Feb 21, 2017
@laanwj
Merge #9813: Read/write mempool.dat as a binary.
7d46b3e 
171fc91 Read/write mempool.dat as a binary. (Pavel Janík)
laanwj pushed a commit that referenced this pull request Feb 21, 2017
@paveljanik @laanwj
Read/write mempool.dat as a binary.
3972a8e 
mempool.dat is a binary file and thus it should be read/written as such.

Fixes #9810.

Github-Pull: #9813
Rebased-From: 171fc91
codablock pushed a commit to codablock/dash that referenced this pull request Jan 26, 2018
@laanwj @codablock
Merge bitcoin#9813: Read/write mempool.dat as a binary.
8d2edc4 
171fc91 Read/write mempool.dat as a binary. (Pavel Janík)
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
@laanwj @andvgal
Merge bitcoin#9813: Read/write mempool.dat as a binary.
4438ff8 
171fc91 Read/write mempool.dat as a binary. (Pavel Janík)
CryptoCentric pushed a commit to absolute-community/absolute that referenced this pull request Feb 27, 2019
@laanwj @CryptoCentric
Merge bitcoin#9813: Read/write mempool.dat as a binary.
10d9ae5 
171fc91 Read/write mempool.dat as a binary. (Pavel Janík)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@gmaxwell gmaxwell gmaxwell approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Mempool Windows

Projects

None yet

Milestone

0.14.0

Development

Successfully merging this pull request may close these issues.

0.14 not loading mempool.dat?

7 participants

@paveljanik @laanwj @theuni @achow101 @fanquake @jonasschnelli @gmaxwell