Function is returning a different value every time?-CodePudding

I'm trying to convert a hexadecimal INT to a char so I could convert it into a binary to count the number of ones in it. Here's my function to convert it into char:

#include <stdio.h>
#include <stdlib.h>
#define shift(a) a=a<<5
#define parity_even(a) a = a 0x11
#define add_msb(a) a = a   8000

void count_ones(int hex){
    char *s = malloc(2);    
    sprintf(s, "0x%x", hex);
    free(s);
    printf("%x", s);
};

int main() {
    int a = 0x01B9;
    shift(a);
    parity_even(a);
    count_ones(a);
    return 0;
}

Every time I run this, i always get different outputs but the first three hex number are always the same. Example of outputs:

8c0ba2a0
fc3b92a0
4500a2a0
d27e82a0
c15d62a0

What exactly is happening here? I allocated 2 bytes for the char since my hex int is 2 bytes.

CodePudding user response：

It's too long to write a comment so here goes:

I'm trying to convert a hexadecimal INT

int are stored as a group of value, padding (possible empty) and sign bits, so is there no such thing as a hexadecimal INT but you can represent (print) a given number in the hexadecimal format.

convert a ... INT to a char

That would be lossy conversion as an int might have 4 bytes of data that you are trying to cram into a 1 byte. char specifically may be signed or unsigned. You probably mean string (generic term) or char [] (standard way to represent a string in C).

binary to count the number of ones

That's the real issue you are trying to solve and this is a duplicate of:

How to count the number of set bits in a 32-bit integer?

count number of ones in a given integer using only << >> | & ^ ~ ! =

To address the question you ask:

Need to allocate more than 2 bytes. Specifically ceil(log16(hex)) 2 (for 0x) 1 (for trailing '\0'). If you want you can use INT_MAX instead of hex to get an upper bound at compile time, by for example using that log16(INT_MAX) = CHAR_BIT * sizeof(int) / 4.

Another way to get the size is to just ask snprintf(s, 0, ...) then allocate a suitable array via malloc or use stack allocated variable length array (VLA).

It's undefined behavior to use a variable after it's deallocated. Move free() to after the last use.

Here is one of the dynamic versions mentioned above:

void count_ones(unsigned hex) {
    char *s = NULL;
    size_t n = snprintf(s, 0, "0x%x", hex)   1;
    s = malloc(n);
    if(!s) return; // memory could not be allocated
    snprintf(s, n, "0x%x", hex);
    printf("%s (size = %zu)", s, n);
    free(s);
};

Note, I initialized s to NULL which would cause the first call to snprintf() to return an undefined value on SUSv2 (legacy). It's well defined on c99 and later. The output is:

0x3731 (size = 7)

And the compile-time version using a fixed upper bound:

#include <limits.h>

// compile-time
void count_ones(unsigned hex) {
    char s[BIT_CHAR * sizeof(int) / 4   3];
    sprintf(s, "0x%x", hex);
    printf("%s (size = %zu)", s, n);
};

and the output is:

0x3731 (size = 11)

CodePudding user response：

Your biggest problem is that malloc isn't allocating enough. As Barmar said, you need at least 7 bytes to store it or you could calculate the amount needed. Another problem is that you are freeing it and then using it. It is only one line after the free that you use it again, which shouldn't have anything bad happen like 99.9% of the time, but you should always free after you know you are done using it.