Confusion about return values after change in libscrypt_scrypt()

[levaidaniel]

Hi!

It seems that after a change in return values in libscrypt_scrypt(), there is still some confusion in the library itself:
In crypto_scrypt-hash.c you do:

retval = libscrypt_scrypt(...)
if(retval == -1)
    return 0;

But after commit 7cd8a4f you don't indicate error with -1 from libscrypt_scrypt(). So there could perfectly be an error with errno set, and this check would evaluate to false, and continue.

But the whole return value change doesn't make much sense to me either, and IMHO, it is a pretty crucial part of the library, and there was not much warning about this change. Even you forgot to follow this deeper into the library. Previously, when error was indicated with -1, the caller could simply check errno if he or she wanted to, and be done with it.
Now you also must ensure in libscrypt_scrypt(), that errno is definitely set before every "goto err0;", otherwise, errno might contain 0, and you end up with an erroneously successful call. (nevermind mind that now every caller must change the retval check for libscrypt_scrypt()...).
I really don't want to be offensive, I'm just wondering about the why. Or maybe I'm just still drunk on christmas cookies, and seeing some things different than they are.

Daniel

[technion]

Hi,

This is a valid issue. The original return value was very clearly -1, however, I was given PR #23 which was accepted I believe, too hastily.

I'm going to revert that PR.

[technion]

Fixed in latest commit.