Amos Jeffries squid3@treenet.co.nz writes:
I think the base64url decoding functions, and its table, should be put in its own object file. Maybe static linking is obscure these days, but it's desirable that if one uses only the standard base64 alphabet, one shouldn't get dependencies causing the linker to drag in tables for other alphabets.
Are you sure you want that?
Except for the init function and table the remainder of the code is all shared. Separating the tables from the code means we would end up with +4 files just holding lookup tables and have to move the table symbols/pointers into a .h.
The idea is that the tables should be statically defined in the file that use it. (I think you did it like that in the updated patch).
I think this works, but only because the ENCODE macro which refers to the name "encode_table" now gets a local variable, instead of the global table in the old code. At a casual look, the local variable appears unused. I think it's easier to understand if this dependency where made more explicit.
Would you like that done as code documentation, manually unrolling ENCODE, or replacing it with a static function?
I think a simple and clear way is to add the table as an argument to the ENCODE macro.
Regards, /Niels