On Fri, Mar 18, 2022 at 02:11:27PM +0600, NRK wrote:
> On Fri, Mar 18, 2022 at 12:40:00AM +0100, Roberto E. Vargas Caballero wrote:
> > Uhmmmm, I didn't realize about it, I just saw that having 255 entries was wrong ^^!!!.
> > I think the best approach is to split the commit in two and evaluate/review
> > them isolated; one commit to fix the size, and other about zeroing.
>
> Digging a bit more into this, I see that the array is only being
> accessed in here, inside base64dec():
>
> while (*src) {
> int a = base64_digits[(unsigned char) base64dec_getc(&src)];
> int b = base64_digits[(unsigned char) base64dec_getc(&src)];
> int c = base64_digits[(unsigned char) base64dec_getc(&src)];
> int d = base64_digits[(unsigned char) base64dec_getc(&src)];
>
> This makes me think that the array should've just been local to the
> function, instead of being declared at file-scope. Anyhow, let's look at
> base64dec_getc():
>
> char
> base64dec_getc(const char **src)
> {
> while (**src && !isprint(**src))
> (*src)++;
> return **src ? *((*src)++) : '='; /* emulate padding if string ends */
> }
>
> Seems like it's only going to return either a printable char, or '='.
> Afaik the highest printable ascii char is 127, so maybe the size of the
> array can be reduced to just 128.
>
> Another thing that comes to mind is that fact that all the <ctype.h>
> functions have bit of a bear-trap in them:
>
> In all cases the argument is an int, the value of which shall be
> representable as an unsigned char or shall equal the value of
> the macro EOF. If the argument has any other value, the behavior
> is undefined.
>
> So maybe **src should also be casted to an unsigned char before passing
> it to isprint().
>
> - NRK
>
I agree for isprint((unsigned char)).
But please make that a separate patch.
Thanks,
--
Kind regards,
Hiltjo
Received on Fri Mar 18 2022 - 10:46:24 CET