Convert hex to base64, part 2

Question

Original question

Reimplemented solution following JS1's answer.

Changed bit string hack to bitwise operations to get the corresponding base64 index values.

#include <stdio.h>
#include <assert.h>

void get_b64_quads(char *hex, char *b64quads, int pad)
{
  char *dec_2_base64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";

  char b1 = hex[0] >> 2;
  char b2 = ((hex[0] & 0x03) << 4) + (hex[1] >> 4);
  char b3 = ((hex[1] & 0x0f) << 2) + (hex[2] >> 6);
  char b4 = hex[2] & 0x3f;

  b64quads[0] = dec_2_base64[b1];
  b64quads[1] = dec_2_base64[b2];
  b64quads[2] = pad == 1 && !b3 ? '=' : dec_2_base64[b3];
  b64quads[3] = pad && !b4 ? '=' : dec_2_base64[b4];

  return;
}

char* hex_2_base64(char *_hex)
{
  int hexstring_size = strlen(_hex);
  if (!hexstring_size)
    return NULL;
  if (hexstring_size % 2 != 0)
    return NULL;

  //remove whitespaces from hex string
  char *moveto = NULL;
  char *p = _hex;
  while (*p)
  {
    if (*p == ' ')
    {
      if (moveto == NULL)
        moveto = p;
    }
    else
    {
      if (moveto)
      {
        memmove(moveto, p, strlen(p) + 1);
        moveto = NULL;
      }
    }
    p++;
  }

  //converts hex string to byte array
  int bytearray_size = hexstring_size / 2; //each pair of hex chars is a byte
  char *bytearray = malloc(bytearray_size);
  p = _hex;
  char *bytearray_p = bytearray;
  char hexbytebuffer[3] = { 0 };
  int temp = hexstring_size;
  int ibytearray = 0;
  while (temp > 0)
  {
    memcpy(hexbytebuffer, p, 2);
    bytearray[ibytearray] = strtol(hexbytebuffer, NULL, 16);
    ibytearray++;
    p += 2;
    temp -= 2;
  }

  //allocate memory for base64 output (must be freed by whoever calls this function)
  int b64length = (((hexstring_size + 2) / 3) * 4) + 1; //each 3 hex bytes will become 4 base64 chars
  char *base64 = malloc(b64length + 1);
  memset(base64, 0, b64length + 1);

  //walk through byte array, converting each 3 bytes to 4 base64 characters
  temp = bytearray_size;
  char *bytearrayp = bytearray;
  char bytes[3] = { 0 };
  char b64chars[4] = { 0 };
  char *base64p = base64;
  while (temp > 0)
  {
    if (temp >= 3)
    {
      memcpy(bytes, bytearrayp, 3);
      get_b64_quads(bytes, b64chars, 0);
      temp -= 3;
      bytearrayp += 3;
    }
    else
    {
      //needs to pad with '='
      memset(bytes, 0, 3);
      memcpy(bytes, bytearrayp, temp);
      get_b64_quads(bytes, b64chars, temp);
      temp = 0;
    }
    memcpy(base64p, b64chars, 4);
    base64p += 4;
  }

  free(bytearray);
  return base64;
}

int main(void) {
  char *out = NULL;

  //I'm killing your brain like a poisonous mushroom
  out = hex_2_base64("49276d206b696c6c696e6720796f757220627261696e206c696b65206120706f69736f6e6f7573206d757368726f6f6d");
  assert(strcmp(out, "SSdtIGtpbGxpbmcgeW91ciBicmFpbiBsaWtlIGEgcG9pc29ub3VzIG11c2hyb29t") == 0);
  free(out);

  //Man is distinguished, not only by his reason, but by this singular passion from other animals, which is a lust of the mind, that by a perseverance of delight in the continued and indefatigable generation of knowledge, exceeds the short vehemence of any carnal pleasure.
  out = hex_2_base64("4d616e2069732064697374696e677569736865642c206e6f74206f6e6c792062792068697320726561736f6e2c2062757420627920746869732073696e67756c61722070617373696f6e2066726f6d206f7468657220616e696d616c732c2077686963682069732061206c757374206f6620746865206d696e642c20746861742062792061207065727365766572616e6365206f662064656c6967687420696e2074686520636f6e74696e75656420616e6420696e6465666174696761626c652067656e65726174696f6e206f66206b6e6f776c656467652c2065786365656473207468652073686f727420766568656d656e6365206f6620616e79206361726e616c20706c6561737572652e");
  assert(strcmp(out, "TWFuIGlzIGRpc3Rpbmd1aXNoZWQsIG5vdCBvbmx5IGJ5IGhpcyByZWFzb24sIGJ1dCBieSB0aGlzIHNpbmd1bGFyIHBhc3Npb24gZnJvbSBvdGhlciBhbmltYWxzLCB3aGljaCBpcyBhIGx1c3Qgb2YgdGhlIG1pbmQsIHRoYXQgYnkgYSBwZXJzZXZlcmFuY2Ugb2YgZGVsaWdodCBpbiB0aGUgY29udGludWVkIGFuZCBpbmRlZmF0aWdhYmxlIGdlbmVyYXRpb24gb2Yga25vd2xlZGdlLCBleGNlZWRzIHRoZSBzaG9ydCB2ZWhlbWVuY2Ugb2YgYW55IGNhcm5hbCBwbGVhc3VyZS4=") == 0);
  free(out);

  //f
  out = hex_2_base64("66");
  assert(strcmp(out, "Zg==") == 0);
  free(out);

  //fo
  out = hex_2_base64("666f");
  assert(strcmp(out, "Zm8=") == 0);
  free(out);

  //foo
  out = hex_2_base64("666f6f");
  assert(strcmp(out, "Zm9v") == 0);
  free(out);

  //foob
  out = hex_2_base64("666f6f62");
  assert(strcmp(out, "Zm9vYg==") == 0);
  free(out);

  //fooba
  out = hex_2_base64("666f6f6261");
  assert(strcmp(out, "Zm9vYmE=") == 0);
  free(out);

  //foobar
  out = hex_2_base64("666f6f626172");
  assert(strcmp(out, "Zm9vYmFy") == 0);
  free(out);

  out = hex_2_base64("00");
  assert(strcmp(out, "AA==") == 0);
  free(out);

  out = hex_2_base64("0000");
  assert(strcmp(out, "AAA=") == 0);
  free(out);

  printf("OK!");

  return 0;
}

Answer 1

Temporary buffers not needed for the most part

I see a lot of places where you copy a few bytes to/from a temporary buffer. I think you could avoid the temporary buffer in most cases and just use the actual source/destination buffer instead. For example, take a look at this main loop in your function:

  while (temp > 0)
  {
    if (temp >= 3)
    {
      memcpy(bytes, bytearrayp, 3);
      get_b64_quads(bytes, b64chars, 0);
      temp -= 3;
      bytearrayp += 3;
    }
    else
    {
      //needs to pad with '='
      memset(bytes, 0, 3);
      memcpy(bytes, bytearrayp, temp);
      get_b64_quads(bytes, b64chars, temp);
      temp = 0;
    }
    memcpy(base64p, b64chars, 4);
    base64p += 4;
  }

There are a few things I would change here. First of all, I take the special case of less than 3 bytes out of the main loop, like this:

  while (temp > 3)
  {
    memcpy(bytes, bytearrayp, 3);
    get_b64_quads(bytes, b64chars, 0);
    memcpy(base64p, b64chars, 4);
    temp -= 3;
    bytearrayp += 3;
    base64p += 4;
  }
  if (temp > 0)
  {
    //needs to pad with '='
    memset(bytes, 0, 3);
    memcpy(bytes, bytearrayp, temp);
    get_b64_quads(bytes, b64chars, temp);
    memcpy(base64p, b64chars, 4);
  }

Second of all, you don't actually need to use the temporary buffers in the main loop. All you are doing in unnecessary copying back and forth. In the special case of less than 3 bytes left, you do need a temporary input buffer because you need to zero out the bytes that don't exit. But you don't need the temporary output buffer. So the code now turns into this:

  while (temp > 3)
  {
    get_b64_quads(bytearrayp, base64p, 0);
    temp       -= 3;
    bytearrayp += 3;
    base64p    += 4;
  }
  if (temp > 0)
  {
    //needs to pad with '='
    uint8_t bytes[3] = {0};
    memcpy(bytes, bytearrayp, temp);
    get_b64_quads(bytes, base64p, temp);
  }

The same thing can be said about the loop that converts the hex string to a byte array:

  while (temp > 0)
  {
    memcpy(hexbytebuffer, p, 2);
    bytearray[ibytearray] = strtol(hexbytebuffer, NULL, 16);
    ibytearray++;
    p += 2;
    temp -= 2;
  }

Here, I would change to this:

  while (temp > 0)
  {
    bytearray[ibytearray++] = (hexval(p[0]) << 4) | hexval(p[1]);
    p    += 2;
    temp -= 2;
  }

where hexval() is a helper function that converts a hex character to a value from 0..15.

Go directly from hex to base64

If you want to be more efficient, you can skip the step where you convert the hex string into a byte array. You can do the base64 conversion directly from the hex string if you want. Right now your last step converts 3 bytes to 4 base64 digits. If you operated on the hex string directly, you would be converting 6 hex characters to 4 base64 digits instead. You'd just need to modify your get_b64_quads() function appropriately.

Sample reimplementation

Here is how it would look like rewritten:

#include <stdio.h>
#include <assert.h>
#include <string.h>
#include <stdlib.h>

static inline int hexVal(char c)
{
    if (c >= 'a' && c <= 'f')
        return 10 + c - 'a';
    if (c >= 'A' && c <= 'F')
        return 10 + c - 'A';
    if (c >= '0' && c <= '9')
        return c - '0';
    return 0;
}

static inline int hexByte(const char *hex)
{
    return (hexVal(hex[0]) << 4) | hexVal(hex[1]);
}

void get_b64_quads(const char *hex, char *b64quads, int pad)
{
  char *dec_2_base64 = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/";

  char h0 = hexByte(hex+0);
  char h1 = hexByte(hex+2);
  char h2 = hexByte(hex+4);
  char b1 = h0 >> 2;
  char b2 = ((h0 & 0x03) << 4) + (h1 >> 4);
  char b3 = ((h1 & 0x0f) << 2) + (h2 >> 6);
  char b4 = h2 & 0x3f;

  b64quads[0] = dec_2_base64[b1];
  b64quads[1] = dec_2_base64[b2];
  b64quads[2] = pad == 2 && !b3 ? '=' : dec_2_base64[b3];
  b64quads[3] = pad && !b4 ? '=' : dec_2_base64[b4];

  return;
}

char* hex_2_base64(char *_hex)
{
  int hexstring_size = strlen(_hex);
  if (!hexstring_size)
    return NULL;
  if (hexstring_size % 2 != 0)
    return NULL;

  //remove whitespaces from hex string
  char *moveto = NULL;
  char *p = _hex;
  while (*p)
  {
    if (*p == ' ')
    {
      if (moveto == NULL)
        moveto = p;
    }
    else
    {
      if (moveto)
      {
        memmove(moveto, p, strlen(p) + 1);
        moveto = NULL;
      }
    }
    p++;
  }
    
  //allocate memory for base64 output (must be freed by whoever calls this function)
  int b64length = (((hexstring_size + 2) / 3) * 4) + 1; //each 3 hex bytes will become 4 base64 chars
  char *base64 = calloc(1, b64length + 1);

  //walk through byte array, converting each 6 hex chars to 4 base64 characters
  char *base64p = base64;
  while (hexstring_size > 6)
  {
      get_b64_quads(_hex, base64p, 0);
      hexstring_size -= 6;
      _hex           += 6;
      base64p        += 4;
  }
  if (hexstring_size > 0)
  {
      char temphex[6] = { '0', '0', '0', '0', '0', '0' };
      memcpy(temphex, _hex, hexstring_size);
      get_b64_quads(temphex, base64p, hexstring_size);
  }
  return base64;
}