jedisct1 / libsodium

A modern, portable, easy to use crypto library.
https://libsodium.org
Other
12.26k stars 1.74k forks source link

"void * const addr" should be "void const *addr" #734

Closed nalzok closed 6 years ago

nalzok commented 6 years ago

I saw these two functions in the document:

int sodium_mlock(void * const addr, const size_t len);
int sodium_munlock(void * const addr, const size_t len);

Since parameters are copied when a function is called, qualifying a parameter with const in function declaration is pointless: you can never modify its original value anyway.

You might want to change it to void const *addr, which means the function will not modify the data addr points to.

Further reading: https://stackoverflow.com/q/51353648/5399734

jedisct1 commented 6 years ago

These functions definitely have side effects on the data at addr, so *addr cannot be const.

The implementation doesn't change addr itself, so it can be const.

nalzok commented 6 years ago

I’m confused. If the secret data (say cleartext password) is changed upon locking its memory, how can I subsequently hash it? In other words, the password I’m hashing may not be what a user have inputted, if there is a side effect introduced by sodium_mlock.

Also, it’s impossible for any implementation to change addr, because C is call by value.

jedisct1 commented 6 years ago

Before a call to sodium_mlock(), *addr can be written to to. After a call to sodium_mlock(), writing to *addr will cause an exception. So it had a side effect on *addr. The behavior wouldn't change if *addr had been const.