Document sodium_init return value

[stouset]

It isn't really clear from the existing documentation that positive values from sodium_init() are considered success. It's easy for a library author (e.g., me) to make the mistake of raising an error if sodium_init() or if sodium_init() != 0, which is a (small) bug.

[jedisct1]

Hi,

The example specifically tests sodium_init() != -1 but indeed, there was no details about the possible return values.

This has been fixed.

Thanks for suggesting this!

[stouset]

That was fast :) Thanks!

[dnaq]

To be honest I believe that sodium_init() should always return 0 when it succeeds. I think it's important that a library exposes consistent error handling, and since all other functions in libsodium (that return ints) return 0 when they succeed sodium_init() should do the same thing. In this case it's not really a big deal, but I'm a firm believer in consistent error handling (and I got bit by this in sodiumoxide).

[jedisct1]

Initializing the library twice is okay if it's done sequentially. This should never be done concurrently.

Calling sodium_init() more than once is more likely to be a bug than intentional. In that case, 0 is the only valid return code for success. Anything else is unexpected.

There are rare exceptions. When writing a quick and dirty prototype without worrying about race conditions. Or where calling sodium_init() more than once can simplify the code, and where the application guarantees that it cannot happen concurrently. For these exceptions, checking for a negative return code can be used to check for failure.

So, we cannot return -1 when library initialization happens more than necessary. This is not always an error.

[dnaq]

Why not -1 and setting errno?

[jedisct1]

That would break existing applications with no real benefits. It's too late for such a change.

[dnaq]

That's probably the right thing to do here, maybe along with deprecating 'sodium_init()' in a future 2.0 release of libsodium. However, I believe that the principle is important for future additions to libsodium. A library that does not have consistent error handling is OpenSSL and I've lost count of the number of times I've seen code that handles OpenSSL errors incorrectly.

When it comes to 'sodium_init()' the consequences of incorrect error handling aren't that big (the only dangerous case is where it returns -1 and that's consistent with the rest of the library). However a new cryptgraphic primitive with different error handling could easily lead to security bugs so it's something worth keeping in mind.

[stouset]

@jedisct1 I disagree that calling sodium_init() more than once is a bug. It's easily conceivable that a program might include two libraries that wrap libsodium and don't know about one-another. I know this, because I am currently writing such libraries (guess how I ran across this documentation issue :smile:).

For this, I disagree that calling sodium_init() more than once should return -1. It's definitely not an error.

However, I'm not sure returning 1 is actually all that useful. In the case where it's being called concurrently from multiple threads, you can't rely on getting a 1 to expose the bug — in fact, buggy behavior due to concurrent calls will only when it returns 0. Given that, in what sort of case is it helpful to know that libsodium was already successfully initialized?