Skip to content
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.

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

Net: Fix resource leak in ReadBinaryFile(...) #10587

Conversation

practicalswift
Copy link
Contributor

@practicalswift practicalswift commented Jun 14, 2017

Potential file descriptor leak introduced in commit 0b6f40d via PR #10408 (submitted 2017-05-16, merged 2017-05-18).

@paveljanik
Copy link
Contributor

ACK f2fb132

@theuni
Copy link
Member

theuni commented Jun 14, 2017

utACK f2fb132, but RAII would be nice here.

@fanquake fanquake added the P2P label Jun 15, 2017
@sipa
Copy link
Member

sipa commented Jun 16, 2017

@theuni Agree, but since the existing code is using fopen/fread/fclose, let's keep using that in the bug fix. The rest can happen later.

utACK f2fb132

@sipa sipa merged commit f2fb132 into bitcoin:master Jun 16, 2017
sipa added a commit that referenced this pull request Jun 16, 2017
f2fb132 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a
@practicalswift
Copy link
Contributor Author

Friendly ping @str4d – you might want this fix in zcash too :-)

@str4d
Copy link
Contributor

str4d commented Jun 23, 2017

Thanks @practicalswift, nice catch!

@laanwj
Copy link
Member

laanwj commented Jun 24, 2017

utACK f2fb132, but RAII would be nice here.

Yes it's pretty easy to make a RAII for FILE* using unique_ptr with a custom deleter, we could even make our fsbridge::fopen return this.
Agree with not doing it in this PR though.

jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jun 28, 2019
Summary:
Merge #10408: Net: Improvements to Tor control port parser
49a199b torcontrol: Handle escapes in Tor QuotedStrings (Jack Grigg)
0182a11 torcontrol: Log invalid parameters in Tor reply strings where meaningful (Jack Grigg)
0b6f40d torcontrol: Check for reading errors in ReadBinaryFile (Jack Grigg)
d63677b torcontrol: Fix ParseTorReplyMapping (Jack Grigg)
29f3c20 torcontrol: Add unit tests for Tor reply parsers (Jack Grigg)
d8e03c0 torcontrol: Improve comments (Jack Grigg)

Tree-SHA512: aa3ce8072d20299b38c4ba9471af7fab1f5df096c237bf40a96ee9274a357f7366f95ced0cc80f8da1f22f6455a1a8e68bad9a5ff71817eef3397b6aefcbc7ae

Backport of Core PR10408
https://github.com/bitcoin/bitcoin/pull/10408/files

Merge #13291: test: Don't include torcontrol.cpp into the test file
97c112d Declare TorReply parsing functions in torcontrol_tests (Ben Woosley)

Pull request description:

  These methods are standalone string parsing methods which were included
  into test via an include of torcontrol.cpp, which is bad practice.

  ~~Splitting them out reveals that they were the only torcontrol.cpp
  methods under test, so the test file is renamed tor_reply_tests.cpp.~~

  Introduced in #10408

Tree-SHA512: 8ff11a9c900a88f910a73dfe16f43581a567e9d60e9298a8a963fc9dd7cffb4d97a644da677610aafb7d89f1dd1cede9afeae2c6344305e021a9a322dbcea0ac

Backport of Core PR13291
bitcoin/bitcoin#13291

(partial) Merge #13163: Make it clear which functions that are intended to be translation unit local
c3f34d0 Make it clear which functions that are intended to be translation unit local (practicalswift)

Pull request description:

  Make it clear which functions that are intended to be translation unit local.

  Do not share functions that are meant to be translation unit local with other translation units. Use internal linkage for those consistently.

Tree-SHA512: 05eebd233d5cfbf6116724eec3a99b465bf534ca220f2b6f5e56341a7da41387454d3cb6ceadd8ab6714a5df94069e4ad0dcab8801ccc7e8949be7199a19fb53

Backport of Core PR13163
bitcoin/bitcoin#13163

Completes T612

Introduces a memory leak fixed by PR10587
bitcoin/bitcoin#10587

Memory leak fix: https://reviews.bitcoinabc.org/D3285

Includes a fix to the locale linter discussed with fabien

D3285 should be landed immediately after this one

Test Plan:
  make check
  test_runner.py
  sudo apt install tor # if not already installed
  sudo etc/init.d/tor status # to check if tor is running
  sudo etc/init.d/tor start # if it is not already running
  ./src/bitcoind -proxy=127.0.0.1:<tor port>
  ./bitcoin-cli getpeerinfo

tor defaults to port 9050.
`getpeerinfo` should be populated.
arc lint should pass with no errors.

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3283
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jun 28, 2019
Summary:
f2fb132 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a

Backport of Core PR10587
bitcoin/bitcoin#10587

Dependent on D3283

Completes T613

This should land immediately after its dependency.

Test Plan:
  make check
  test_runner.py

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3285
jtoomim pushed a commit to jtoomim/bitcoin-abc that referenced this pull request Jun 29, 2019
Summary:
Merge #10408: Net: Improvements to Tor control port parser
49a199b torcontrol: Handle escapes in Tor QuotedStrings (Jack Grigg)
0182a11 torcontrol: Log invalid parameters in Tor reply strings where meaningful (Jack Grigg)
0b6f40d torcontrol: Check for reading errors in ReadBinaryFile (Jack Grigg)
d63677b torcontrol: Fix ParseTorReplyMapping (Jack Grigg)
29f3c20 torcontrol: Add unit tests for Tor reply parsers (Jack Grigg)
d8e03c0 torcontrol: Improve comments (Jack Grigg)

Tree-SHA512: aa3ce8072d20299b38c4ba9471af7fab1f5df096c237bf40a96ee9274a357f7366f95ced0cc80f8da1f22f6455a1a8e68bad9a5ff71817eef3397b6aefcbc7ae

Backport of Core PR10408
https://github.com/bitcoin/bitcoin/pull/10408/files

Merge #13291: test: Don't include torcontrol.cpp into the test file
97c112d Declare TorReply parsing functions in torcontrol_tests (Ben Woosley)

Pull request description:

  These methods are standalone string parsing methods which were included
  into test via an include of torcontrol.cpp, which is bad practice.

  ~~Splitting them out reveals that they were the only torcontrol.cpp
  methods under test, so the test file is renamed tor_reply_tests.cpp.~~

  Introduced in #10408

Tree-SHA512: 8ff11a9c900a88f910a73dfe16f43581a567e9d60e9298a8a963fc9dd7cffb4d97a644da677610aafb7d89f1dd1cede9afeae2c6344305e021a9a322dbcea0ac

Backport of Core PR13291
bitcoin/bitcoin#13291

(partial) Merge #13163: Make it clear which functions that are intended to be translation unit local
c3f34d0 Make it clear which functions that are intended to be translation unit local (practicalswift)

Pull request description:

  Make it clear which functions that are intended to be translation unit local.

  Do not share functions that are meant to be translation unit local with other translation units. Use internal linkage for those consistently.

Tree-SHA512: 05eebd233d5cfbf6116724eec3a99b465bf534ca220f2b6f5e56341a7da41387454d3cb6ceadd8ab6714a5df94069e4ad0dcab8801ccc7e8949be7199a19fb53

Backport of Core PR13163
bitcoin/bitcoin#13163

Completes T612

Introduces a memory leak fixed by PR10587
bitcoin/bitcoin#10587

Memory leak fix: https://reviews.bitcoinabc.org/D3285

Includes a fix to the locale linter discussed with fabien

D3285 should be landed immediately after this one

Test Plan:
  make check
  test_runner.py
  sudo apt install tor # if not already installed
  sudo etc/init.d/tor status # to check if tor is running
  sudo etc/init.d/tor start # if it is not already running
  ./src/bitcoind -proxy=127.0.0.1:<tor port>
  ./bitcoin-cli getpeerinfo

tor defaults to port 9050.
`getpeerinfo` should be populated.
arc lint should pass with no errors.

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3283
jtoomim pushed a commit to jtoomim/bitcoin-abc that referenced this pull request Jun 29, 2019
Summary:
f2fb132 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a

Backport of Core PR10587
bitcoin/bitcoin#10587

Dependent on D3283

Completes T613

This should land immediately after its dependency.

Test Plan:
  make check
  test_runner.py

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3285
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 5, 2019
f2fb132 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 5, 2019
f2fb132 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a
jonspock pushed a commit to jonspock/devault that referenced this pull request Jul 6, 2019
Summary:
f2fb132cb Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a

Backport of Core PR10587
bitcoin/bitcoin#10587

Dependent on D3283

Completes T613

This should land immediately after its dependency.

Test Plan:
  make check
  test_runner.py

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3285
jonspock pushed a commit to jonspock/devault that referenced this pull request Jul 6, 2019
Summary:
Merge #10408: Net: Improvements to Tor control port parser
49a199b torcontrol: Handle escapes in Tor QuotedStrings (Jack Grigg)
0182a11 torcontrol: Log invalid parameters in Tor reply strings where meaningful (Jack Grigg)
0b6f40d torcontrol: Check for reading errors in ReadBinaryFile (Jack Grigg)
d63677b torcontrol: Fix ParseTorReplyMapping (Jack Grigg)
29f3c20 torcontrol: Add unit tests for Tor reply parsers (Jack Grigg)
d8e03c0 torcontrol: Improve comments (Jack Grigg)

Tree-SHA512: aa3ce8072d20299b38c4ba9471af7fab1f5df096c237bf40a96ee9274a357f7366f95ced0cc80f8da1f22f6455a1a8e68bad9a5ff71817eef3397b6aefcbc7ae

Backport of Core PR10408
https://github.com/bitcoin/bitcoin/pull/10408/files

Merge #13291: test: Don't include torcontrol.cpp into the test file
97c112d4ca Declare TorReply parsing functions in torcontrol_tests (Ben Woosley)

Pull request description:

  These methods are standalone string parsing methods which were included
  into test via an include of torcontrol.cpp, which is bad practice.

  ~~Splitting them out reveals that they were the only torcontrol.cpp
  methods under test, so the test file is renamed tor_reply_tests.cpp.~~

  Introduced in #10408

Tree-SHA512: 8ff11a9c900a88f910a73dfe16f43581a567e9d60e9298a8a963fc9dd7cffb4d97a644da677610aafb7d89f1dd1cede9afeae2c6344305e021a9a322dbcea0ac

Backport of Core PR13291
bitcoin/bitcoin#13291

(partial) Merge #13163: Make it clear which functions that are intended to be translation unit local
c3f34d06be Make it clear which functions that are intended to be translation unit local (practicalswift)

Pull request description:

  Make it clear which functions that are intended to be translation unit local.

  Do not share functions that are meant to be translation unit local with other translation units. Use internal linkage for those consistently.

Tree-SHA512: 05eebd233d5cfbf6116724eec3a99b465bf534ca220f2b6f5e56341a7da41387454d3cb6ceadd8ab6714a5df94069e4ad0dcab8801ccc7e8949be7199a19fb53

Backport of Core PR13163
bitcoin/bitcoin#13163

Completes T612

Introduces a memory leak fixed by PR10587
bitcoin/bitcoin#10587

Memory leak fix: https://reviews.bitcoinabc.org/D3285

Includes a fix to the locale linter discussed with fabien

D3285 should be landed immediately after this one

Test Plan:
  make check
  test_runner.py
  sudo apt install tor # if not already installed
  sudo etc/init.d/tor status # to check if tor is running
  sudo etc/init.d/tor start # if it is not already running
  ./src/bitcoind -proxy=127.0.0.1:<tor port>
  ./bitcoin-cli getpeerinfo

tor defaults to port 9050.
`getpeerinfo` should be populated.
arc lint should pass with no errors.

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3283
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 6, 2019
f2fb132 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a
jonspock pushed a commit to jonspock/devault that referenced this pull request Jul 7, 2019
Summary:
f2fb132cb Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a

Backport of Core PR10587
bitcoin/bitcoin#10587

Dependent on D3283

Completes T613

This should land immediately after its dependency.

Test Plan:
  make check
  test_runner.py

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3285
jonspock pushed a commit to jonspock/devault that referenced this pull request Jul 7, 2019
Summary:
Merge #10408: Net: Improvements to Tor control port parser
49a199b torcontrol: Handle escapes in Tor QuotedStrings (Jack Grigg)
0182a11 torcontrol: Log invalid parameters in Tor reply strings where meaningful (Jack Grigg)
0b6f40d torcontrol: Check for reading errors in ReadBinaryFile (Jack Grigg)
d63677b torcontrol: Fix ParseTorReplyMapping (Jack Grigg)
29f3c20 torcontrol: Add unit tests for Tor reply parsers (Jack Grigg)
d8e03c0 torcontrol: Improve comments (Jack Grigg)

Tree-SHA512: aa3ce8072d20299b38c4ba9471af7fab1f5df096c237bf40a96ee9274a357f7366f95ced0cc80f8da1f22f6455a1a8e68bad9a5ff71817eef3397b6aefcbc7ae

Backport of Core PR10408
https://github.com/bitcoin/bitcoin/pull/10408/files

Merge #13291: test: Don't include torcontrol.cpp into the test file
97c112d4ca Declare TorReply parsing functions in torcontrol_tests (Ben Woosley)

Pull request description:

  These methods are standalone string parsing methods which were included
  into test via an include of torcontrol.cpp, which is bad practice.

  ~~Splitting them out reveals that they were the only torcontrol.cpp
  methods under test, so the test file is renamed tor_reply_tests.cpp.~~

  Introduced in #10408

Tree-SHA512: 8ff11a9c900a88f910a73dfe16f43581a567e9d60e9298a8a963fc9dd7cffb4d97a644da677610aafb7d89f1dd1cede9afeae2c6344305e021a9a322dbcea0ac

Backport of Core PR13291
bitcoin/bitcoin#13291

(partial) Merge #13163: Make it clear which functions that are intended to be translation unit local
c3f34d06be Make it clear which functions that are intended to be translation unit local (practicalswift)

Pull request description:

  Make it clear which functions that are intended to be translation unit local.

  Do not share functions that are meant to be translation unit local with other translation units. Use internal linkage for those consistently.

Tree-SHA512: 05eebd233d5cfbf6116724eec3a99b465bf534ca220f2b6f5e56341a7da41387454d3cb6ceadd8ab6714a5df94069e4ad0dcab8801ccc7e8949be7199a19fb53

Backport of Core PR13163
bitcoin/bitcoin#13163

Completes T612

Introduces a memory leak fixed by PR10587
bitcoin/bitcoin#10587

Memory leak fix: https://reviews.bitcoinabc.org/D3285

Includes a fix to the locale linter discussed with fabien

D3285 should be landed immediately after this one

Test Plan:
  make check
  test_runner.py
  sudo apt install tor # if not already installed
  sudo etc/init.d/tor status # to check if tor is running
  sudo etc/init.d/tor start # if it is not already running
  ./src/bitcoind -proxy=127.0.0.1:<tor port>
  ./bitcoin-cli getpeerinfo

tor defaults to port 9050.
`getpeerinfo` should be populated.
arc lint should pass with no errors.

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3283
jonspock pushed a commit to jonspock/devault that referenced this pull request Jul 7, 2019
Summary:
f2fb132cb Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a

Backport of Core PR10587
bitcoin/bitcoin#10587

Dependent on D3283

Completes T613

This should land immediately after its dependency.

Test Plan:
  make check
  test_runner.py

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3285
jonspock pushed a commit to jonspock/devault that referenced this pull request Jul 7, 2019
Summary:
Merge #10408: Net: Improvements to Tor control port parser
49a199b torcontrol: Handle escapes in Tor QuotedStrings (Jack Grigg)
0182a11 torcontrol: Log invalid parameters in Tor reply strings where meaningful (Jack Grigg)
0b6f40d torcontrol: Check for reading errors in ReadBinaryFile (Jack Grigg)
d63677b torcontrol: Fix ParseTorReplyMapping (Jack Grigg)
29f3c20 torcontrol: Add unit tests for Tor reply parsers (Jack Grigg)
d8e03c0 torcontrol: Improve comments (Jack Grigg)

Tree-SHA512: aa3ce8072d20299b38c4ba9471af7fab1f5df096c237bf40a96ee9274a357f7366f95ced0cc80f8da1f22f6455a1a8e68bad9a5ff71817eef3397b6aefcbc7ae

Backport of Core PR10408
https://github.com/bitcoin/bitcoin/pull/10408/files

Merge #13291: test: Don't include torcontrol.cpp into the test file
97c112d4ca Declare TorReply parsing functions in torcontrol_tests (Ben Woosley)

Pull request description:

  These methods are standalone string parsing methods which were included
  into test via an include of torcontrol.cpp, which is bad practice.

  ~~Splitting them out reveals that they were the only torcontrol.cpp
  methods under test, so the test file is renamed tor_reply_tests.cpp.~~

  Introduced in #10408

Tree-SHA512: 8ff11a9c900a88f910a73dfe16f43581a567e9d60e9298a8a963fc9dd7cffb4d97a644da677610aafb7d89f1dd1cede9afeae2c6344305e021a9a322dbcea0ac

Backport of Core PR13291
bitcoin/bitcoin#13291

(partial) Merge #13163: Make it clear which functions that are intended to be translation unit local
c3f34d06be Make it clear which functions that are intended to be translation unit local (practicalswift)

Pull request description:

  Make it clear which functions that are intended to be translation unit local.

  Do not share functions that are meant to be translation unit local with other translation units. Use internal linkage for those consistently.

Tree-SHA512: 05eebd233d5cfbf6116724eec3a99b465bf534ca220f2b6f5e56341a7da41387454d3cb6ceadd8ab6714a5df94069e4ad0dcab8801ccc7e8949be7199a19fb53

Backport of Core PR13163
bitcoin/bitcoin#13163

Completes T612

Introduces a memory leak fixed by PR10587
bitcoin/bitcoin#10587

Memory leak fix: https://reviews.bitcoinabc.org/D3285

Includes a fix to the locale linter discussed with fabien

D3285 should be landed immediately after this one

Test Plan:
  make check
  test_runner.py
  sudo apt install tor # if not already installed
  sudo etc/init.d/tor status # to check if tor is running
  sudo etc/init.d/tor start # if it is not already running
  ./src/bitcoind -proxy=127.0.0.1:<tor port>
  ./bitcoin-cli getpeerinfo

tor defaults to port 9050.
`getpeerinfo` should be populated.
arc lint should pass with no errors.

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3283
proteanx pushed a commit to devaultcrypto/devault that referenced this pull request Jul 7, 2019
Summary:
f2fb132cb Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a

Backport of Core PR10587
bitcoin/bitcoin#10587

Dependent on D3283

Completes T613

This should land immediately after its dependency.

Test Plan:
  make check
  test_runner.py

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3285
proteanx pushed a commit to devaultcrypto/devault that referenced this pull request Jul 7, 2019
Summary:
Merge #10408: Net: Improvements to Tor control port parser
49a199b torcontrol: Handle escapes in Tor QuotedStrings (Jack Grigg)
0182a11 torcontrol: Log invalid parameters in Tor reply strings where meaningful (Jack Grigg)
0b6f40d torcontrol: Check for reading errors in ReadBinaryFile (Jack Grigg)
d63677b torcontrol: Fix ParseTorReplyMapping (Jack Grigg)
29f3c20 torcontrol: Add unit tests for Tor reply parsers (Jack Grigg)
d8e03c0 torcontrol: Improve comments (Jack Grigg)

Tree-SHA512: aa3ce8072d20299b38c4ba9471af7fab1f5df096c237bf40a96ee9274a357f7366f95ced0cc80f8da1f22f6455a1a8e68bad9a5ff71817eef3397b6aefcbc7ae

Backport of Core PR10408
https://github.com/bitcoin/bitcoin/pull/10408/files

Merge #13291: test: Don't include torcontrol.cpp into the test file
97c112d4ca Declare TorReply parsing functions in torcontrol_tests (Ben Woosley)

Pull request description:

  These methods are standalone string parsing methods which were included
  into test via an include of torcontrol.cpp, which is bad practice.

  ~~Splitting them out reveals that they were the only torcontrol.cpp
  methods under test, so the test file is renamed tor_reply_tests.cpp.~~

  Introduced in #10408

Tree-SHA512: 8ff11a9c900a88f910a73dfe16f43581a567e9d60e9298a8a963fc9dd7cffb4d97a644da677610aafb7d89f1dd1cede9afeae2c6344305e021a9a322dbcea0ac

Backport of Core PR13291
bitcoin/bitcoin#13291

(partial) Merge #13163: Make it clear which functions that are intended to be translation unit local
c3f34d06be Make it clear which functions that are intended to be translation unit local (practicalswift)

Pull request description:

  Make it clear which functions that are intended to be translation unit local.

  Do not share functions that are meant to be translation unit local with other translation units. Use internal linkage for those consistently.

Tree-SHA512: 05eebd233d5cfbf6116724eec3a99b465bf534ca220f2b6f5e56341a7da41387454d3cb6ceadd8ab6714a5df94069e4ad0dcab8801ccc7e8949be7199a19fb53

Backport of Core PR13163
bitcoin/bitcoin#13163

Completes T612

Introduces a memory leak fixed by PR10587
bitcoin/bitcoin#10587

Memory leak fix: https://reviews.bitcoinabc.org/D3285

Includes a fix to the locale linter discussed with fabien

D3285 should be landed immediately after this one

Test Plan:
  make check
  test_runner.py
  sudo apt install tor # if not already installed
  sudo etc/init.d/tor status # to check if tor is running
  sudo etc/init.d/tor start # if it is not already running
  ./src/bitcoind -proxy=127.0.0.1:<tor port>
  ./bitcoin-cli getpeerinfo

tor defaults to port 9050.
`getpeerinfo` should be populated.
arc lint should pass with no errors.

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3283
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 8, 2019
f2fb132 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a
jonspock pushed a commit to devaultcrypto/devault that referenced this pull request Jul 9, 2019
Summary:
f2fb132cb Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a

Backport of Core PR10587
bitcoin/bitcoin#10587

Dependent on D3283

Completes T613

This should land immediately after its dependency.

Test Plan:
  make check
  test_runner.py

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3285
jonspock pushed a commit to devaultcrypto/devault that referenced this pull request Jul 9, 2019
Summary:
Merge #10408: Net: Improvements to Tor control port parser
49a199b torcontrol: Handle escapes in Tor QuotedStrings (Jack Grigg)
0182a11 torcontrol: Log invalid parameters in Tor reply strings where meaningful (Jack Grigg)
0b6f40d torcontrol: Check for reading errors in ReadBinaryFile (Jack Grigg)
d63677b torcontrol: Fix ParseTorReplyMapping (Jack Grigg)
29f3c20 torcontrol: Add unit tests for Tor reply parsers (Jack Grigg)
d8e03c0 torcontrol: Improve comments (Jack Grigg)

Tree-SHA512: aa3ce8072d20299b38c4ba9471af7fab1f5df096c237bf40a96ee9274a357f7366f95ced0cc80f8da1f22f6455a1a8e68bad9a5ff71817eef3397b6aefcbc7ae

Backport of Core PR10408
https://github.com/bitcoin/bitcoin/pull/10408/files

Merge #13291: test: Don't include torcontrol.cpp into the test file
97c112d4ca Declare TorReply parsing functions in torcontrol_tests (Ben Woosley)

Pull request description:

  These methods are standalone string parsing methods which were included
  into test via an include of torcontrol.cpp, which is bad practice.

  ~~Splitting them out reveals that they were the only torcontrol.cpp
  methods under test, so the test file is renamed tor_reply_tests.cpp.~~

  Introduced in #10408

Tree-SHA512: 8ff11a9c900a88f910a73dfe16f43581a567e9d60e9298a8a963fc9dd7cffb4d97a644da677610aafb7d89f1dd1cede9afeae2c6344305e021a9a322dbcea0ac

Backport of Core PR13291
bitcoin/bitcoin#13291

(partial) Merge #13163: Make it clear which functions that are intended to be translation unit local
c3f34d06be Make it clear which functions that are intended to be translation unit local (practicalswift)

Pull request description:

  Make it clear which functions that are intended to be translation unit local.

  Do not share functions that are meant to be translation unit local with other translation units. Use internal linkage for those consistently.

Tree-SHA512: 05eebd233d5cfbf6116724eec3a99b465bf534ca220f2b6f5e56341a7da41387454d3cb6ceadd8ab6714a5df94069e4ad0dcab8801ccc7e8949be7199a19fb53

Backport of Core PR13163
bitcoin/bitcoin#13163

Completes T612

Introduces a memory leak fixed by PR10587
bitcoin/bitcoin#10587

Memory leak fix: https://reviews.bitcoinabc.org/D3285

Includes a fix to the locale linter discussed with fabien

D3285 should be landed immediately after this one

Test Plan:
  make check
  test_runner.py
  sudo apt install tor # if not already installed
  sudo etc/init.d/tor status # to check if tor is running
  sudo etc/init.d/tor start # if it is not already running
  ./src/bitcoind -proxy=127.0.0.1:<tor port>
  ./bitcoin-cli getpeerinfo

tor defaults to port 9050.
`getpeerinfo` should be populated.
arc lint should pass with no errors.

Reviewers: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Reviewed By: jasonbcox, deadalnix, Fabien, O1 Bitcoin ABC, #bitcoin_abc

Differential Revision: https://reviews.bitcoinabc.org/D3283
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 9, 2019
f2fb132 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jul 9, 2019
f2fb132 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a
barrystyle pushed a commit to PACGlobalOfficial/PAC that referenced this pull request Jan 22, 2020
f2fb132 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Tree-SHA512: 879b9334d8bb681fa4b6f96d8ecb54e2a8948065f7be5fe7880131479c813602fc9d4a4314f043e6591e1aed50ffafa7c247362a9cdeb049b0721170e227b89a
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Apr 4, 2020
a9c922b Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Pull request description:

  Straightforward fix. Coming from upstream [10587](bitcoin#10587)
  ```
  Potential file descriptor leak introduced in commit 0b6f40d via PR bitcoin#10408
  (submitted 2017-05-16, merged 2017-05-18).
  ```

ACKs for top commit:
  Fuzzbawls:
    ACK a9c922b
  random-zebra:
    utACK a9c922b and merging...

Tree-SHA512: d9fdf1f04e73bb2118a2ad88113d2f4676dedba8668fc1396f446689000f1a75a8bb678f5f84cb7be20781bde1932f2ffe8606666fd371d00d2d5228077350c6
wqking pushed a commit to wqking-temp/Vitae that referenced this pull request Sep 4, 2020
a9c922b96bd0b17c61e2b8742d72ef5bc894ebb8 Net: Fix resource leak in ReadBinaryFile(...) (practicalswift)

Pull request description:

  Straightforward fix. Coming from upstream [10587](bitcoin/bitcoin#10587)
  ```
  Potential file descriptor leak introduced in commit 0b6f40d via PR #10408
  (submitted 2017-05-16, merged 2017-05-18).
  ```

ACKs for top commit:
  Fuzzbawls:
    ACK a9c922b96bd0b17c61e2b8742d72ef5bc894ebb8
  random-zebra:
    utACK a9c922b96bd0b17c61e2b8742d72ef5bc894ebb8 and merging...

Tree-SHA512: d9fdf1f04e73bb2118a2ad88113d2f4676dedba8668fc1396f446689000f1a75a8bb678f5f84cb7be20781bde1932f2ffe8606666fd371d00d2d5228077350c6
@practicalswift practicalswift deleted the fopen-not-followed-by-fclose-in-all-states-of-the-universe branch April 10, 2021 19:31
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants