Hello all!Im trying to write a program that accepts a string from users and then converts each character in the string to its corresponding 8-bit binary code and store it for further manipulation on obtained bit. I have the following code but it is not executing as expected :

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

int xstrlen (char *s) //for later use
{
int length = 0 ;
while ( *s != '\0' )
{
length++ ;
s++ ;
}
return ( length ) ;
}

char* showbits(int n) //to print binary equivalent
{
int i, k, andmask ;

char* p=(char *)malloc(9); //8 bytes for 8 bit binary and 1byte for null character at end
for ( i = 7 ; i >= 0 ; i-- )
{
    andmask = 1 << i ;
    k = n & andmask ;

    if(k==0)
    {
       strcat(p,"0");
    }

    else
    {
       strcat(p,"1");
    }
}
*ptr='\0'; //dont know if its correct
return p;
}


void msg2bits(char *msg)
{

    char* bits;
    int x;
    int i=0;


   while(*msg!='\0')
   {
       x=(int)msg[i];
       bits=showbits(x);

       puts(bits);
       msg++;
       i++;
    }
}


int main(int argc,char *argv[])
{
    char *msg="ABCD";

    msg2bits(msg);
    getch();
    return 0;
}

Dani AI

Generated

Good progress by — the crash from the first post was caused by using strcat on an uninitialised buffer and was correctly addressed by initializing the buffer. Two remaining issues (the ones was hinting at) are important and worth fixing properly.

First: memory-management. In the revised msg2bits you allocate bits and then immediately overwrite that pointer with the return from showbits, leaking the originally allocated block. On top of that, showbits itself allocates a fresh buffer for every character and none of those buffers are freed. Fix this by avoiding per-character heap allocation: either allocate one reusable buffer on the stack or let the caller supply a buffer. That removes leaks and is simpler and faster.

Second: signedness. Converting char to int directly can produce negative values on platforms where char is signed; bitwise operations then give surprising results. Use (unsigned char) (or mask with 0xFF) when forming the integer whose bits you want to inspect.

A safe, minimal pattern (no per-character malloc, no strcat, correct signedness) is to let the caller pass a 9-byte buffer and fill it by index. For example:

void showbits(unsigned char ch, char out[9]) {
    for (int i = 0; i < 8; ++i)
        out[i] = (ch & (1 << (7 - i))) ? '1' : '0';
    out[8] = '\0';
}

Then in the loop use a single char bits[9]; and call showbits((unsigned char)*msg, bits); puts(bits);.

Other small points: always check malloc when you must use it, avoid non-portable getch() (use getchar() or remove), and prefer stack buffers for tiny fixed-size strings. These changes address the two glaring errors and make the routine robust and portable.

finally solved the problem. here is the modified code:-

char* showbits(int n)
{
int i, k, andmask ;

char* p=malloc(9*sizeof(char));
*p='\0';
for ( i = 7 ; i >= 0 ; i-- )
{
    andmask = 1 << i ;
    k = n & andmask ;

    if(k==0)
    {
       strcat(p,"0");
    }

    else
    {
       strcat(p,"1");
    }
}

return p;
}


void msg2bits(char *msg)
{
    char *bits=malloc(9*sizeof(char));
    int x;

   while(*msg!='\0')
   {
       x=(int)msg[0];
       bits=showbits(x);
       puts(bits);
       msg++;
    }
}

Well done. However, you have two glaring errors in the modified code. See if you can find them.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.