Base64 encoding implementation

I am learning C and decided to make an implementation of Base64 encoding according to the info Wikipedia provides about it.

My main question is: should I declare the index_table array inside the get_base64_digit function like static char index_table[] = "blabla"?

A general code review would also be appreciated.

funcs.c

static char index_table[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";

static char get_index_digit(int x) {
    return index_table[x];
}

char get_base64_digit(const unsigned int base64, int digit_number){

    switch (digit_number){
            case 0:
                    return get_index_digit(base64 >> 18);
            case 1:
                    return get_index_digit((base64 >> 12) & 077);
            case 2:
                    return get_index_digit((base64 >> 6) & 077);
            case 3:
                    return get_index_digit(base64 & 077);
    }

    return '\0';
}

funcs.h

#include 
#include 

char get_base64_digit(unsigned int base64, int digit_number);

main.c

```
#include "funcs.h"

int main(int argc, char* argv[]){

char* text = argc == 2 ? argv[1] : '\0';

unsigned final, temp;

int text_len = strlen(text);
int remainder = text_len % 3;

//Iterate over the input string, three characters at a time.
for (int x = final = temp = 0; x < (remainder == 0 ? text_len : text_len - remainder); x += 3) {

final = (text[x] << 16) | (text[x+1] << 8) | text[x+2];

printf("%c%c%c%c", get_base64_digit(final, 0), get_base64_digit(final, 1), get_base64_digit(final, 2), get_base64_digit(final, 3));

}

//Handle the last bytes in case text_len wasn't a multiple of 3
if (remainder == 1) {
final = text[text_len-1] << 16;
printf("%c%c==\n", get_base64_digit(final, 0), get_base64_digit(final, 1));
} else if (remainder == 2){
final = (text[text_len-2] << 16) | (text[text_len-1] << 8);
print

funcs.h

Neither funcs.c nor funcs.h require anything from stdio.h or string.h, so you shouldn't be including those there, move them to main.c instead. But you should get into the habit of adding include guards to your headers. So either use this at the top:

#pragma once

Or, to be completely standard and portable:

#ifndef FUNCS_H
#define FUNCS_H

// contents of header here

#endif // FUNCS_H

funcs.c

The static encode mapping is fine as you have it, but would also be fine inside the get_index_digit function (static also of course). Since that is the only direct user, it would be slightly better inside in my opinion.

I'd change your signature and preamble for the encoder to:

#include 
// ...

char get_base64_digit(const unsigned int base64, unsigned int digit_number)
{
  assert(digit_number < 4);
  // ...

Passing a negative index or anything greater than 3 is a programming error, it should be caught as early as possible and be a hard failure.

You can get rid of the switch by collapsing all cases:

return get_index_digit((base64 >> (18 - 6*digit_number)) & 077);

Note that this masks the bits even for the zero case, which is safer. But you might want instead to assert on the fact that the bits above index 18 are all zeros - slightly different contract. (I'd prefer just masking them off though.)

main.c

char* text = argc == 2 ? argv[1] : '\0';

That '\0' is bad. Happens to work, but doesn't convey the right information. text is a pointer, not a char. Replace with:

char* text = argc == 2 ? argv[1] : NULL;

But that's actually not a good idea either. What you actually need is an empty string (or bail out).

const char* text = argc == 2 ? argv[1] : "";

Then you have:

unsigned final, temp;

But you're not using temp anywhere, except for setting it to zero. And you're always initializing final to zero in the for loop. So to help simplifying that for a bit:

unsigned final = 0;

Also a slight inconsistency: you're using unsigned here, but used unsigned int in the declaration for get_base64_digit - pick one version and stick to that (they are completely identical).

As for the loop:

for (int x = final = temp = 0; x < (remainder == 0 ? text_len : text_len - remainder); x += 3) {

That line is too long, and more complex than it needs to be. Removing final and temp will help:

for (int x = 0; x < (remainder == 0 ? text_len : text_len - remainder); x += 3) {

And might help you to see that the conditional is unnecessary. If remainder is zero, text_len and text_len - remainder have the same value. So:

for (int x = 0; x < text_len - remainder; x += 3) {

Or move that subtraction out of the loop for an even simpler structure.

The last lines are a bit odd:

} else
        return 0;

return 0;

Remove the else, it is not useful. Also mixing braced and non-braced blocks in an if/else chain isn't a good idea. This is a matter of style, but always using braces is usually safer. If you want to omit them for single-statement blocks, only do that if you only have single-statement blocks in the if/else chain.

Last tidbit: you sometimes have ){ (no space), and sometimes ) { (one space). Stick to one, preferably the one with the space. You also sometimes have an empty line at the start of blocks, sometimes not. If you feel like a bit of vertical separation is good there, try putting the { that starts the block on its own line (as I did in the get_base64_digit above), and look at the Linux coding style guidelines for some other tips that you may, or may not :-), like.

clang-format is an excellent auto-format tool, and a lot of dev tools (including Vim and Emacs) can be made to use it. Try it out.

StackExchange Code Review Q#119841, answer score: 4