Skip to content

Conversation

practicalswift
Copy link
Contributor

No description provided.

@TheBlueMatt
Copy link
Contributor

I'd vote we just remove the dead code.

@fanquake
Copy link
Member

I'd vote we just remove the dead code.

Agree.

@JeremyRubin
Copy link
Contributor

NACK. Code seems to be reachable?

JSONRPCError doesn't inherit from std::exception. transformNamedArguments could throw a JSONRPCError.

@TheBlueMatt
Copy link
Contributor

TheBlueMatt commented Jan 19, 2017 via email

@laanwj
Copy link
Member

laanwj commented Jan 19, 2017

This clearly seems a bug. There's two options here:

  • repair the PostCommand functionality so that the PostCommand handler actually runs (by having a RAII cleanup that triggers it)
  • remove the PostCommand functionality

It looks like this currently influences nothing as no PostCommand handlers are being registered.

@practicalswift
Copy link
Contributor Author

practicalswift commented Feb 28, 2017

@laanwj Updated and pushed. I simply removed the PostCommand functionality as suggested. Let me know if you have any further suggestions! :-)

@practicalswift practicalswift changed the title [trivial] Add comment about unreachable code Remove unreachable code Feb 28, 2017
@laanwj
Copy link
Member

laanwj commented Mar 9, 2017

@practicalswift You should more thoroughly remove it then. Also remove the signal. Otherwise people will register it in the expectation it will be called.

@practicalswift practicalswift force-pushed the never-executed-comment branch from 9478e44 to 54fae05 Compare March 9, 2017 08:47
@practicalswift
Copy link
Contributor Author

@laanwj Another try - looks good? :-)

@laanwj
Copy link
Member

laanwj commented Mar 9, 2017

Yes, all the traces of it should be gone. Looks good to me now.
(also renamed the pull to more clearly summarize what it does)

@laanwj laanwj changed the title Remove unreachable code Remove unused, non-working RPC PostCommand signal Mar 9, 2017
@laanwj laanwj merged commit 54fae05 into bitcoin:master Mar 9, 2017
laanwj added a commit that referenced this pull request Mar 9, 2017
54fae05 Remove unreachable code (g_rpcSignals.PostCommand) (practicalswift)

Tree-SHA512: b416d63ffa1c35fa0434b6bc0a930ae1b04919a12114ef65154ce4621aa43d744eb4fc515f48eac2e96c0f528cedb4cdbe2349952d215c2c74e900a63e155e0a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 21, 2019
54fae05 Remove unreachable code (g_rpcSignals.PostCommand) (practicalswift)

Tree-SHA512: b416d63ffa1c35fa0434b6bc0a930ae1b04919a12114ef65154ce4621aa43d744eb4fc515f48eac2e96c0f528cedb4cdbe2349952d215c2c74e900a63e155e0a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 27, 2019
54fae05 Remove unreachable code (g_rpcSignals.PostCommand) (practicalswift)

Tree-SHA512: b416d63ffa1c35fa0434b6bc0a930ae1b04919a12114ef65154ce4621aa43d744eb4fc515f48eac2e96c0f528cedb4cdbe2349952d215c2c74e900a63e155e0a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Jan 29, 2019
54fae05 Remove unreachable code (g_rpcSignals.PostCommand) (practicalswift)

Tree-SHA512: b416d63ffa1c35fa0434b6bc0a930ae1b04919a12114ef65154ce4621aa43d744eb4fc515f48eac2e96c0f528cedb4cdbe2349952d215c2c74e900a63e155e0a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 5, 2019
54fae05 Remove unreachable code (g_rpcSignals.PostCommand) (practicalswift)

Tree-SHA512: b416d63ffa1c35fa0434b6bc0a930ae1b04919a12114ef65154ce4621aa43d744eb4fc515f48eac2e96c0f528cedb4cdbe2349952d215c2c74e900a63e155e0a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 5, 2019
54fae05 Remove unreachable code (g_rpcSignals.PostCommand) (practicalswift)

Tree-SHA512: b416d63ffa1c35fa0434b6bc0a930ae1b04919a12114ef65154ce4621aa43d744eb4fc515f48eac2e96c0f528cedb4cdbe2349952d215c2c74e900a63e155e0a
PastaPastaPasta pushed a commit to PastaPastaPasta/dash that referenced this pull request Feb 5, 2019
54fae05 Remove unreachable code (g_rpcSignals.PostCommand) (practicalswift)

Tree-SHA512: b416d63ffa1c35fa0434b6bc0a930ae1b04919a12114ef65154ce4621aa43d744eb4fc515f48eac2e96c0f528cedb4cdbe2349952d215c2c74e900a63e155e0a
Fuzzbawls added a commit to PIVX-Project/PIVX that referenced this pull request Feb 14, 2021
84b4185 [Tests] raise minimum fee in feature_blockindexstats (random-zebra)
39d8a20 [Cleanup] Remove OldSetKeyFromPassphrase/OldEncrypt/OldDecrypt (random-zebra)
613e1da [BUG] Fix sizeof in paymentservertests (random-zebra)
0f3e961 [Cleanup] Remove useless call (random-zebra)
b1ca5e0 Remove unused fsbridge::freopen (practicalswift)
b3a1d84 Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) (practicalswift)
ecfbcfd [Cleanup] Remove unused function AddInvalidSpendsToMap (random-zebra)
15b018c [Refactor] Use InsecureRand in the unit tests (random-zebra)
81fd84c [Refactoring] Replace risky call to rand() with GetRandInt() (random-zebra)
8ba1978 [Qt] Terminate string *pszExePath after readlink and without using memset (practicalswift)
b6cd719 Remove unreachable code (g_rpcSignals.PostCommand) (practicalswift)
e4f9ab3 [BUG] Memory leak after new CNode / ConnectNode (random-zebra)
536122b Avoid rollingMinimumFeeRate never being able to decay below half (Alex Morcos)
13cad19 fix a bug if the min fee is 0 for FeeFilterRounder (Alex Morcos)
aa832d8 [Refactoring] Dereference before/after null check (random-zebra)
4630b7d [Trivial] Pass big parameters by reference, not value (random-zebra)
aa7bc7f [Refactoring] Remove unneeded CScript::IsNormalPaymentScript (random-zebra)
6cf3c8f [Trivial] Unitialized scalar fields and variables (random-zebra)

Pull request description:

  This is a collection of small fixes for the following issues/defects (discovered with the coverity tool):

  - Big parameters passed by value
  - calls to rand() function
  - Dereference before/after null-pointer check
  - Pointer to local variable out of scope
  - Resource leak with call to ConnectNode
  - Non-floating point result - unintended integer division (bitcoin#9288)
  - String not-null-terminated (bitcoin#11193)
  - Structurally dead code (bitcoin#9575)
  - Unchecked boolean return value of functions (GetOp, GetTransaction, TxOutToPublicCoin)
  - Unitialized pointers and scalar fields
  - Illegal memory access (bitcoin#13159)
  - Useless call to pubkey.IsCompressed()
  - Wrong `sizeof` argument (bitcoin#11024 + revert #494)

ACKs for top commit:
  furszy:
    Looking good, ACK 84b4185
  Fuzzbawls:
    ACK 84b4185

Tree-SHA512: 4c920a1858ccde65795c090e4becc47c50c0a78db7ab11935b4d3e2bea3f7f1f8ca3b48ce633d8112673092c35ae7cd05c444ed2b16e283a305c765874e55c1c
@practicalswift practicalswift deleted the never-executed-comment branch April 10, 2021 19:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants