Convert a binary string, with each char separated by spaces, into a normal C string and print

Question

This code is a purely for-fun project and it comes from this challenge at freeCodeCamp. This is simply an algorithm challenge which is usually meant for JavaScript, but I decided to perform in C. It will not be used for any production reasons. Please leave any feedback you see fit, with special attention to the loops and data types. I purposely used pointer arithmetic, while, and for loops to explore different approaches to the problems. I also left the printf about number of tokens in there if you'd like to play with it by uncommenting. Also note that I've provided 2 sample binary strings but only one is currently used by default.

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

char* binary_agent(char *bin_str);
char bit_string_decode(char *const str);
bool bit_string_is_valid(char *bit_string);
int main(void)
{
    char str[] = "01000001 01110010 01100101 01101110 00100111 01110100 00100000 "
                 "01100010 01101111 01101110 01100110 01101001 01110010 01100101 "
                 "01110011 00100000 01100110 01110101 01101110 00100001 00111111";
    char str2[] = "01001001 00100000 01101100 01101111 01110110 01100101 00100000 01000110 01110010 01100101 01100101 01000011 01101111 "
                  "01100100 01100101 01000011 01100001 01101101 01110000 00100001";
    char *result = binary_agent(str);
    printf("%s\n", result);
    free(result);
    result = NULL;
    return 0;
}

// Purpose: Return an English translated sentence of the passed binary string
// "Chars" must be space-separated
// E.G. binary_agent("01000001 01110010 01100101");
char* binary_agent(char *bin_str)
{
    assert(bin_str);

    size_t count;
    char *token = NULL;
    char *delim = " ";
    count = 0;
    char *ptr = bin_str;

    while((ptr = strchr(ptr, ' ')) != NULL)
    {
        count++;
        ptr++;
    }

    ptr = NULL;
    //printf("Total number of tokens %lu\n", count);

    char *output_buffer = malloc(sizeof(char)*(count+1));
    assert(output_buffer);

    token = strtok(bin_str,delim); // token is null-terminated
    if(!token) return NULL;
    output_buffer[0] = bit_string_decode(token);
    size_t i;
    for(i = 1; token != NULL; ++i) //convert to for loop
    {
        token = strtok(NULL, delim);
        if(!token) break;
        output_buffer[i] = bit_string_decode(token);
    }
    return output_buffer;
}

//Parses groups of 8 1s or 0s into a char
char bit_string_decode(char *const str)
{
    assert(str && bit_string_is_valid(str));

    char output = 0;
    size_t i;

    //String needs to be valid for sure here
    for(i = 0; i < 8; ++i)
    {
        switch(str[i])
        {
            case '1':
                output |= 1 << 7-i;
                break;
            case '0':
                break;
        }
    }

    return output;
}
bool bit_string_is_valid(char *bit_string)
{
    assert(bit_string && (strlen(bit_string) == 8));

    while(bit_string && *bit_string != '\0')
    {
        if((*bit_string != '0') && (*bit_string != '1')) return false;
        bit_string++;
    }
    return true;
}

Answer 1

OP: Please leave any feedback you see fit, with special attention to the loops and data types.

---

char type

Although char is signed or unsigned, string functions of the standard library work with the data as if it is was unsigned char.

... each character shall be interpreted as if it had the type unsigned char ... C17dr §7.24.1 3

Also: char output = 0; ... output |= 1 << 7-i; is or-ing bits potentially into the sign place. This really is not a concern, but in general, logical bit manipulation is best done with unsigned types.

int/unsigned, in general, is the preferred type to use for efficient/clean code. Thus using unsigned/int rather than char output can make for better code.

Putting these ideas together: recommend something more like

char bit_string_decode(char *const str) {
    unsigned output = 0;
    size_t i;
    for(i = 0; i < 8; ++i) {
        switch(str[i]) {
            case '1':
                output |= 1u << 7-i;  // I prefer unsigned types for bit manipulations
                break;
            case '0':
                break;
        }
    }
    return (char) output;
}

const

bit_string_decode(char *const str) does not modify the reference data. Use const for greater functional usage, more function clarity and help weak compilers optimize.

// bit_string_decode(char *const str)
bit_string_decode(const char *const str)
//                ^---^

// bool bit_string_is_valid(char *bit_string)
bool bit_string_is_valid(const char *bit_string)

Simplifications

Some alternative ideas to simplify the loop:

char bit_string_decode(const char *str) {
    const unsigned char *ustr = (const unsigned char *) str;
    unsigned output = 0;
    while (*(const unsigned char*)ustr) { // cast: see note below
        output <<= 1;
        // Given *str is '0' or '1'
        output |= *ustr++ - '0';
    }
    return (char) output;
}

Allocate to the size of the referenced data, not type

Code is easier to code right, review and maintain.

// char *output_buffer = malloc(sizeof(char)*(count+1));
char *output_buffer = malloc(sizeof *output_buffer *(count+1));
//                                  ^------------^  Referenced data

Consider decoding without destruction

binary_agent(char *bin_str) messes up bin_str due to strtok(bin_str,delim). Use of strspn(), strcspn(), strchr() could be used to find the delimiter without changing bin_str. This functional change I find would make for more useful code and greater applicability.

Missing validation in production

I would say strlen(bit_string) == 8 (or equivalent) should always test, not in only in assert. Perhaps as

bool bit_string_is_valid(const char *bit_string) {
  if (bit_string == NULL) {
    return false;
  }
  const char *s = bit_string;
  while(* (const unsigned char*)s) {  // cast: see note below
    if((*s != '0') && (*s != '1')) {
      return false;
    }
    s++;
  }
  return (s - bit_string) == 8;
}

The cast (const unsigned char*) is only useful here for the all but extinct non-2's complement platforms to properly ID the null character. Drop it, unless code needs that level of portability.

---

Minor

Spacing

Separate function declarations from functions a bit.

bool bit_string_is_valid(char *bit_string);
// add space here
int main(void)