-1 is not a valid member of type size_t

[HalosGhost]

The encrypt function exposed returns type size_t, but the documentation specifies that it might return -1 if the operations failed. This will just result in returning SIZE_MAX which means a successful operation returning the maximum size is indistinguishable from failure.

Is there a reason you wanted to avoid an extra out-param for the resulting cipher-text size, freeing the return value to be something like PORTABLE8439_{OK, ERROR} instead?

[DavyLandman]

I get your point, but I do want to point out you can never in a valid scenario get SIZE_MAX, as the size of the cipher text is always RFC_8439_TAG_SIZE longer than the decrypted result.

portable8439/src/portable8439.c

Line 106 in 4d11ef4

size_t actual_size = cipher_text_size - RFC_8439_TAG_SIZE;

Regarding the design, I think I based it on existing cryptography libraries. I think I prefer not to use extra out parameters. Maybe the result should be boolean. and the user has to subtract the tag size from the actual size themselves (which they already need to do to allocate enough buffer for the decryption output.

[HalosGhost]

Perhaps I'm misunderstanding, but shouldn't it be possible to have a message of size SIZE_MAX - RFC_8439_TAG_SIZE, where the resulting ciphertext size would be SIZE_MAX? If so—recognizing that's… unlikely to happen on any modern, non-embedded architecture—wouldn't that lead to what would appear to be a failure case?

[DavyLandman]

Ah, true, I only thought about decryption. With encryption -1 is truly an edge case. Only if you pass in overlapping pointers (not respecting the restrict).

Maybe the size thing should be two macros/functions you can use. And the function can return true/false.

[HalosGhost]

I know it's a breaking API change, but I think if we should have learned anything from NULL (obviously, opinions are polarized), it's that separating error reporting from the valid {co,}domains of API calls is probably a good idea. ☺

[DavyLandman]

Sure, I'm just pointing out it's an edge case, but I'm open to changing it.

Maybe instead of a boolean it should return an error code. to leave room for details (like reason for failure) and 2 defines to help the user calculate the size of the output buffer.

[HalosGhost]

That sounds like a really good idea to me. Plus, enumerated error codes make APIs much clearer in my opinion.