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

Variables that are meant to be 0 shouldn't be explicitly initialised to it #782

Closed
Rishabh42 opened this issue Feb 2, 2021 · 3 comments
Closed

Comments

@Rishabh42
Copy link

Rishabh42 commented Feb 2, 2021

I read the following suggestion for uninitialized variables in the detector doc:

If a variable is meant to be initialized to zero, explicitly set it to zero.

This not the best solution to deal with this as Solidity sets the value to 0 by default, if a variable is not initialized.
Moreover, explicitly initializing a variable (that is meant to be =0), to zero just costs more gas.

Following are it's occurences:

  1. https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-local-variables
  2. https://github.com/crytic/slither/wiki/Detector-Documentation#uninitialized-state-variables

Please make the desired changes.
Thanks

@montyly
Copy link
Member

montyly commented Feb 8, 2021

Hi @Rishabh42.

We recommend to always explicitly set the variable to its default value. This is because when reviewing something like

function f() public{
    unit my_var;
   // [..]
}

The reader cannot determine if my_var is supposed to be zero or not. While doing

function f() public{
    unit my_var = 0;
  // [..]
}

Make the code more readable and avoid ambiguities.

If you want to remove these results manually, you can add // slither-disable-next-line uninitialized-local and // slither-disable-next-line uninitialized-state before the variables.

@Rishabh42
Copy link
Author

Thanks for your advice. In that case, I think the doc should specify that the above mentioned suggestion is intended to improve code readability

@montyly
Copy link
Member

montyly commented Mar 7, 2021

Hi @Rishabh42

We updated the documentation, the online documentation will be updated with the upcoming 0.7.1 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants