Feature request: Improve quality of function prototypes

[fefe17]

libsodium started from nacl, which had the unique selling point of having a very clear API with self-explanatory function prototypes. Example from their docs:

 #include "crypto_box.h"
 
 unsigned char pk[crypto_box_PUBLICKEYBYTES];
 unsigned char sk[crypto_box_SECRETKEYBYTES];
 
 crypto_box_keypair(pk,sk);

You can't really do anything wrong here. Here is what the libsodium prototype declaration looks like:

    int crypto_box_keypair(unsigned char *pk, unsigned char *sk)
                __attribute__ ((nonnull));

If you look at the implementation, you will find:

  int
  crypto_box_seed_keypair(unsigned char *pk, unsigned char *sk,
                          const unsigned char *seed)
  {
      return crypto_box_curve25519xsalsa20poly1305_seed_keypair(pk, sk, seed);
  }

The compiler has no idea how large the destination buffer needs to be, and thus can't warn if it is too small. The declaration should instead look like this:

int crypto_box_keypair(unsigned char pk[crypto_box_PUBLICKEYBYTES], unsigned char sk[crypto_box_SECRETKEYBYTES]);

Going further, many libsodium functions take pointer + length style arguments. Those can be annotated as well. Here is how you'd do it with gcc:

  int
  crypto_box_afternm(unsigned char *c, const unsigned char *m,
                     unsigned long long mlen, const unsigned char *n,
                     const unsigned char *k)
  __attribute__((access(read_only,2,3)))

Unfortunately this annotation only works in recent-ish gcc versions, not in clang.
Use #if __has_attribute(access) to guard against breakage.

The upside of doing this is that the compiler will tell you if you call things wrong, for example if you call a function on 100 bytes from a buffer that is only 50 bytes long. Also, if you tell gcc that you are planning to read from a buffer, it can warn if the buffer is not initialized.

[jedisct1]

int crypto_box_keypair(unsigned char pk[crypto_box_PUBLICKEYBYTES], unsigned char sk[crypto_box_SECRETKEYBYTES]);

static should be added before these lengths.

Something like that was done in the past, but caused issues and had to be reverted. Pointers and arrays are not interchangeable.

[fefe17]

I did the work to make a patch. I tried attaching it here but it looks like github ate it. If it did, you can find it under https://ptrace.fefe.de/libsodium-gcc-attribute.patch

The whole test suite still passes (but you get a few new warnings compiling it in what looks like purposefully written tests to break protocol) with both gcc 15 and clang 20.1.0 and clang 21. clang does not get the benefit from the access attribute though, but the build and test suite still work.

I'm leaving this here in case you reconsider. I will use this patch to find bugs in a project that uses libsodium.
Building with this patch also revealed that the internal calls in libsodium are consistent, which is good news.
It has already paid off for me. Do with it what you will.

I'm curious what kind of issues you are talking about. Do you still have logs or bugs in the tracker about the previous attempt?