Re: [RFC PATCH v7 02/24] fscrypt: export fscrypt_base64_encode and fscrypt_base64_decode

[Eric Biggers <ebiggers@kernel.org>]

Some nits about comments:

On Fri, Jun 25, 2021 at 09:58:12AM -0400, Jeff Layton wrote:
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 6ca7d16593ff..32b1f50433ba 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -178,10 +178,8 @@ static int fname_decrypt(const struct inode *inode,
>  static const char lookup_table[65] =
>  	"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+,";
>  
> -#define BASE64_CHARS(nbytes)	DIV_ROUND_UP((nbytes) * 4, 3)
> -
>  /**
> - * base64_encode() - base64-encode some bytes
> + * fscrypt_base64_encode() - base64-encode some bytes
>   * @src: the bytes to encode
>   * @len: number of bytes to encode
>   * @dst: (output) the base64-encoded string.  Not NUL-terminated.
>   *
>   * Encodes the input string using characters from the set [A-Za-z0-9+,].
>   * The encoded string is roughly 4/3 times the size of the input string.
>   *
>   * Return: length of the encoded string
>   */
> -static int base64_encode(const u8 *src, int len, char *dst)
> +int fscrypt_base64_encode(const u8 *src, int len, char *dst)

As this function will be used more widely, this comment should be fixed to be
more precise.  "Roughly 4/3" isn't precise; it's actually exactly
FSCRYPT_BASE64_CHARS(len), right?  The following would be better:

 * Encode the input bytes using characters from the set [A-Za-z0-9+,].
 *
 * Return: length of the encoded string.  This will be equal to
 *         FSCRYPT_BASE64_CHARS(len).

> +/**
> + * fscrypt_base64_decode() - base64-decode some bytes
> + * @src: the bytes to decode
> + * @len: number of bytes to decode
> + * @dst: (output) decoded binary data

It's a bit confusing to talk about decoding "bytes"; it's really a string.
How about:

 * fscrypt_base64_decode() - base64-decode a string
 * @src: the string to decode
 * @len: length of the source string, in bytes
 * @dst: (output) decoded binary data
 *
 * Decode a string that was previously encoded using fscrypt_base64_encode().
 * The string doesn't need to be NUL-terminated.

> + * Return: length of the decoded binary data

Also the error return values should be documented, e.g.:

 * Return: length of the decoded binary data, or a negative number if the source
 *         string isn't a valid base64-encoded string.

- Eric