Re: Add support for base64url variant

[pierre.techoueyres@free.fr (Pierre Téchoueyres)]

Noam Postavsky <npostavs@gmail.com> writes:

> On Thu, 23 May 2019 at 13:51, Pierre Téchoueyres
> <pierre.techoueyres@free.fr> wrote:
>>
>> Eli Zaretskii <eliz@gnu.org> 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.
>
This could be an option ... but I do see this as ugly adding characters
on one side to remove them from the other.

Maybe it could be better to split the functions then ? What about

(base64-encode-string STRING &optional NO-LINE-BREAK)
(base64url-encode-string STRING &optional NO-PAD)

and

(base64-encode-region BEG END &optional NO-LINE-BREAK)
(base64url-encode-region BEG END &optional NO-PAD)



>> 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.

Thanks, I hoped for something like your answer.