jwerle / b64.c Goto Github PK

Hi,

I found your code really good. It seems that the current implementation follows the RFC 4648 standard.

Could you please confirm that it is the case ? If so, could you update the ReadMe so it makes clear which standard you are implementing to everyone ?

Thanks !

Hi,

in your function you're writing this:

enc = realloc(size);
enc[size++] = something;

This is wrong. If you allocates n byte, you cannot access the nth first bytes of the allocated array (think using size = 1).
You need to allocate n + 1. And check the end of the function, you have another invalid write there
(just use b64_encode with valgrind).

Have fix that using static strings.

I prefer a function with this signature :

/*
 * Encode some data in b64.
 * @param src Pointer to the content to encode
 * @param len Content's length
 * @param dst Pointer to the encoded result string (NULL-terminated)
 * @param dst_size Maximal size of the dst content.
 * @return Some error if `dst_size` is lower than the required decoded content length.
 */
int b64_encode(const char* src, size_t len, char* dst, size_t dst_size);

This way your implementation doesn't allocate anything by itself, discharging this responsibility to the caller.

How about making this library able to use static buffers?
I am playing with them now on this branch. For now I've added b64_encode_static because it has different parameters set, but ideally would be to make one entry point with optional (nullable) parameter for output buffer, so b64_encode would look like this.
Another thing would be introduce some macro (eg B64_STATIC_BUFFERS_ONLY) which would exclude code for dynamic memory allocation. Have a look at nanopb library. Quite big, but in can run both with dynamic memory allocation and without (excluded at compilation time).

Motivation for this is performance improvement - I am currently making project which is translating some serial protocol into another (with human readable frames) so that calling malloc and free for every single incoming byte seems to be overhead for this task.

b64_decode is not handling white spaces. I have explicitly added code in my source code to ignore white spaces. Please add it.

Makefile build failed:

make
cc -std=c99 -Wall -Ideps -c -o test.o test.c
test.c:12:19: fatal error: ok/ok.h: No such file or directory
compilation terminated.
: recipe for target 'test.o' failed
make: *** [test.o] Error 1

buffer.c solved a problem that I was having with reallocs when doing a stress test, I'm not sure what the problem was, but it doesn't happen anymore 👍. So, I was looking at the implementation, and I believe that we may have problems when dealing with multhread application.

The buffer control variable is in a global context, which can cause problems between threads generating base64 at the same time and need different buffer sizes.

I thought if it would be interesting if the variable bufc stayed in the scope of the method, or, the buffer was a struct that would contain the information from the buffer.

I believe the simplest implementation would be something like:

int b64_buf_malloc(char * buf);
char* b64_buf_realloc(unsigned char* ptr, size_t size, int * buf_size);

Or

typedef struct buffer {
    char * ptr;
    int bufc;
} buffer_t;

buffer_t * b64_buf_malloc();
void b64_buf_realloc(buffer_t * buffer , size_t size);

I can act on it if it is an acceptable solution.

Line 44 of decode.c

if (!(isalnum(src[j]) || '+' == src[j] || '/' == src[j])) { break; }

Should be

if (!(isalnum(**(int)**src[j]) || '+' == src[j] || '/' == src[j])) { break; }

Otherwise an error: array subscript has type 'char' [-Werror=char-subscripts] occurs. Referring to GCC 9s ctype.h:

/* These macros are intentionally written in a manner that will trigger
a gcc -Wall warning if the user mistakenly passes a 'char' instead
of an int containing an 'unsigned char'. Note that the sizeof will
always be 1, which is what we want for mapping EOF to __CTYPE_PTR[0];
the use of a raw index inside the sizeof triggers the gcc warning if
__c was of type char, and sizeof masks side effects of the extra __c.
Meanwhile, the real index to __CTYPE_PTR+1 must be cast to int,
since isalpha(0x100000001LL) must equal isalpha(1), rather than being
an out-of-bounds reference on a 64-bit machine. */