Re: Add support for base64url variant

[Noam Postavsky]

On Thu, 23 May 2019 at 13:51, Pierre Téchoueyres
<address@hidden> wrote:
>
> Eli Zaretskii <address@hidden> writes:
>
> >> ...
> >> I would like to have some discussion on how to improve some points :
> >> - Is adding parameter to existing functions the way to go or is it
> >> better to add new ones for base64url
> >
> > I think an optional argument will be fine.
>
> Except here I've added two, and one that doesn't really mean anything if
> the second isn't set.  ie. you should not (at least without breaking the
> RFC) generate an base64 string without padding.  At first many
> parameters seemed to me a good thing : limitted patch and flexibility.
> But now I'm no more sure ...

You could leave out the NO-PAD argument, it's easy enough for the
calling Lisp code to delete a couple of "=" chars if really needed.

> I would also bring your attention on the part where I dynamically assign
> pointers on specialized arrays for encoding (resp. decoding).
>
> ex: line 244 of patch
>
> char const *b64_value_to_char = (url_variant) ? base64url_value_to_char : 
> base64_value_to_char;
>
>
> Before my change there were static const, so I suppose compiler could
> have inlined them or at least stored on some cache.  But now ...
> So I'm a little scarried by the possible lost of performance.  If anyone
> has some hint on how I could benchmark this

I doubt it will have a measurable impact on performance. And even if
it did have some very tiny effect, what would you do about it?
Duplicate the whole encoding function, just to avoid a dynamic choice
of array? The difference would have to be pretty drastic to be worth
that, IMO.