Amos Jeffries squid3@treenet.co.nz writes:
The attached patch implements a proposed API/ABI extension adding support for RFC 4648 section 5 "Base 64 Encoding with URL and Filename Safe Alphabet"
Thanks. I have some comments below. Also, testcases are essential (and I suspect that the definition of base64_encode_group is not really accepted by the compiler).
For the standardized alphabets external code simply calls the init() function relevant to the alphabet it is needing to encode/decode with. The library internally uses the context to select which lookup table to use for later base64 function calls.
Or we could introduce a generalized init function, if that need arises.
-#define ENCODE(x) (encode_table[0x3F & (x)]) +#define ENCODE(x) (ctx->alphabet[0x3F & (x)])
I think
#define ENCODE(alphabet, x) ((alphabet)[0x3F & (x)])
is better, not relying on "ctx" variable being defined where the macro is used.
void base64_encode_raw(uint8_t *dst, size_t length, const uint8_t *src) {
- struct base64_encode_ctx ctx;
- base64_encode_init(&ctx);
- _base64_encode_raw(&cxt, dst, length, src);
+}
I think it would be better to let _base64_encode_raw take the alphabet as an argument. base64_encode_raw would then be simply
void base64_encode_raw(uint8_t *dst, size_t length, const uint8_t *src) { _base64_encode_raw(default_encode_table, dst, length, src); }
void base64_encode_group(uint8_t *dst, uint32_t group) {
- struct base64_encode ctx;
- base64_encode_init(&ctx);
- *dst++ = ENCODE(group >> 18); *dst++ = ENCODE(group >> 12); *dst++ = ENCODE(group >> 6);
@@ -144,6 +111,7 @@ void
And similarly here, I think it would be better to not use any ctx struct, and instead do
*dst++ = ENCODE(default_encode_table, group >> 18); *dst++ = ENCODE(default_encode_table, group >> 12); *dst++ = ENCODE(default_encode_table, group >> 6);
(And this function is of questionable utility...)
Regards, /Niels