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

[Jeff Layton <jlayton@kernel.org>]

On Mon, 2021-07-12 at 09:22 -0500, Eric Biggers wrote:
> On Mon, Jul 12, 2021 at 07:55:37AM -0400, Jeff Layton wrote:
> > On Sun, 2021-07-11 at 12:40 -0500, Eric Biggers wrote:
> > > 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).
> > > 
> > 
> > I'm not certain, but I thought that FSCRYPT_BASE64_CHARS gave you a
> > worst-case estimate of the inflation. This returns the actual length of
> > the resulting encoded string, which may be less than
> > FSCRYPT_BASE64_CHARS(len).
> > 
> 
> As far as I can tell, it's the exact amount.

Yeah, now that I went back and re-read the code, I think you're right.
I'll fix the comment.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>