Cannot print char* typed field of typedef struct

Question

The following code converts decimal into binary and stores its binary representation into struct. get_binary_representation() works correctly which you can see for yourself by uncomenting line that prints each bit of val. However when I attempt to print binary representation stored inside binary nothing is being printed.

#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <inttypes.h>
#include <stdbool.h>


typedef struct {
    char* val;
    int   length;
} binary;


char* get_binary_representation(uint16_t val, int length) {
    
    char* result; 

    if (length < 1)
        return NULL;

    if (val == 0) {
        result = (char*) malloc(sizeof(char) * 2);
        result[0] = '0';
        result[1] = '\0';
        return result;
    }

    result = (char*) malloc(sizeof(char) * length);
    result[length - 1] = '\0';

    for (int i = length; i > 0; i--) {
        result[i] = ((val & 1) == 1) ? '1' : '0';
        // printf("Bit %d = %c\n", i, result[i]);
        val >>= 1;
    }

    return result;
}


binary i16_to_binary(uint16_t* val) {
    binary result; // tried replacing it with pointer and using malloc, didn't help
    result.val = get_binary_representation(*val, 16);    
    /* There is a function that finds length of number in binary 
       but I excluded it from MRE for simplicity sake. 
       16 for length because uint16_t is 16 bit integer*/
    return result;
}


int main(int argc, char** argv) {
    uint16_t a = 16; uint16_t b = 254; uint16_t c = 37;
    
    binary a_bin = i16_to_binary(&a);
    binary b_bin = i16_to_binary(&b);
    binary c_bin = i16_to_binary(&c);
    
    /* should print 0000 0000 0001 0000, 0000 0000 0111 1110, 0000 0000 0010 0101
       without spaces and commas */
    printf("%s %s %s\n", a_bin.val, b_bin.val, c_bin.val);

    return 0;
}

MikeCAT · Accepted Answer · 2021-07-16T14:18:57.300

Be careful:

Don't cause buffer overrun. Only indice 0 to length-1 (both inclusive) are available for array with length elements. result[i] is out-of-range when i = length.
Initialize all elements to read. reuslt[0] is not initalized.
Casting results of malloc() family is considered as a bad practice.

The part:

    if (val == 0) {
        result = (char*) malloc(sizeof(char) * 2);
        result[0] = '0';
        result[1] = '\0';
        return result;
    }

    result = (char*) malloc(sizeof(char) * length);
    result[length - 1] = '\0';

    for (int i = length; i > 0; i--) {
        result[i] = ((val & 1) == 1) ? '1' : '0';
        // printf("Bit %d = %c\n", i, result[i]);
        val >>= 1;
    }

Should be:

    if (val == 0) {
        result = malloc(sizeof(char) * 2); /* no casting the result */
        result[0] = '0';
        result[1] = '\0';
        return result;
    }

    result = malloc(sizeof(char) * (length + 1)); /* no casting the result and allocate enough elements */
    result[length] = '\0'; /* put NUL to proper place */

    for (int i = length - 1; i >= 0; i--) { /* adjust the indice */
        result[i] = ((val & 1) == 1) ? '1' : '0';
        // printf("Bit %d = %c\n", i, result[i]);
        val >>= 1;
    }

Cannot print char* typed field of typedef struct

1 Answers1