Re: [hackers] [st][PATCH] rm unnecessary explicit zeroing

From: NRK <nrk_AT_disroot.org>
Date: Fri, 18 Mar 2022 14:11:27 +0600

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
Received on Fri Mar 18 2022 - 09:11:27 CET