Skip to content

Conversation

khorben
Copy link
Contributor

@khorben khorben commented Mar 20, 2012

Hi bitcoin team,

I have just made this trivial change on my system (NetBSD/amd64 6.0_BETA) so that compilation warnings are more relevant. Without it, each file compiled triggers this message:
cc1plus: warning: -Wformat-security ignored without -Wformat

FWIW, "gcc --version" returns:
gcc (NetBSD nb2 20110806) 4.5.3

Cheers,
-- khorben

@gavinandresen
Copy link
Contributor

I could've sworn -Wextra included -Wformat. Maybe it does on later versions of gcc....

@laanwj
Copy link
Member

laanwj commented Mar 21, 2012

ACK

gavinandresen added a commit that referenced this pull request Mar 21, 2012
The generic UNIX Makefile builds with lots of warnings
@gavinandresen gavinandresen merged commit c2b1ab0 into bitcoin:master Mar 21, 2012
coblee referenced this pull request in litecoin-project/litecoin Jul 17, 2012
The generic UNIX Makefile builds with lots of warnings
ptschip pushed a commit to ptschip/bitcoin that referenced this pull request Feb 13, 2018
Makefile.am: Typo fix that broke `make clean` rule
lateminer pushed a commit to lateminer/bitcoin that referenced this pull request Oct 30, 2019
7261134 [Review] Convert runtime_error to JSONRPCError (Cave Spectre)
f7ac53b [RPC] Correct issues with budget commands (Cave Spectre)

Pull request description:

  ### **Release notes**
  - [RPC] Fix potential wallet crashes in budget commands
  - [RPC] Remove unnecessary conditionals in parameter checking of budget commands

  ### **Details**
  **wallet segfault**
  `preparebudget`, invoked before the wallet has fully started, caused a segfault and crashed.  This was due to the lack of checking for `pwalletMain` before referencing cs_wallet.  This is solved with a throw error to tell the user to try again after the wallet has started.

  Furthermore; both `preparebudget` and `submitbudget` has a check for `pindexPrev` to conditionalize the assignment of nBlockMin (which references `pindexPrev->nHeight`); however later checks, `pindexPrev->nHeight` is referenced even if the pindexPrev check failed; which would have caused a crash if pindexPrev was NULL.  This would occur if `chainActive->Tip()` returns null.  Given the logic of `getnextsuperblock`, the check added to preparebudget and submitbudget will generate a throw error if there is no active chain tip.

  **Remove unnecessary conditionals and code**
  `pindexPrev` is loaded from `chainActive.Tip()` pindexPrev->nHeight is checked multiple times in the parameter checking.  For ease of reading, pindexPrev->nHeight is now saved in a variable as it is used multiple times in the execution of the RPC commands in question.  Likewise, two function calls are used to determine the constant length of a budget cycle `Params().GetBudgetCycleBlocks()`; and is used several times.  Storing this information in a constant is more efficient then several calls for the information.

  nBlockMin is determined (the next superblock).  However later nNext was defined to be the exact same thing.  It's not necessary to build them both.

  By nature of validating that the chosen budget cycle block is greater than the current block, and validating the number of cycles is greater than zero; the need to check the end of the budget cycle is unnecessary.

  Having a separate throw message for choosing the wrong block (not a budget block), and specifying a block that has passed is unnecessary.  These have been combined so that the user, putting in the wrong block, is informed, in both cases, what the next budget block is.

  ### **Note**
  This PR was originally part of bitcoin#964, but split out to distinguish between these changes and the other's refactoring.

ACKs for top commit:
  random-zebra:
    ACK PIVX-Project@7261134
  Fuzzbawls:
    ACK 7261134

Tree-SHA512: b0ccabae52d02767971104353b48d4efb76926017d7099b52b341d525710aa3a70d343c92c260048bb1c288990a914730a4dd0832578f07e96abb384d628cd4e
@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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants