Let's consider your code step by step.
First of all the function should be declared like
char * addBinary( const char *s1, const char *s2 );
because passed strings are not changed within the function.
Secondly, the function strlen
has the return type size_t
. You should also use this unsigned ingteger type instead of the signed type int
.
Thirdly, in this call of calloc
char *result = calloc( Max(lengthA, lengthB)+2, sizeof(char) );
you are allocating one more byte when it is not required . At first you need to check whether there is an overflow and only in this case to allocate one more byte.
Also in this for loop
for( int i = Max(lengthA, lengthB); i >= 0; i-- )
{
int bit = carry;
if(idx_a >= 0) bit += a[idx_a--] -'0';
if(idx_b >= 0) bit += b[idx_b--] -'0';
carry = bit>>1;
result[i] = (bit%2) + '0';
}
there is a redundant iteration when there is no an overfflow. For example if you have two strings like a = "0"
, b = "1"
then there are two iterations for i
equal to 1
and to 0
while you need to add two characters only one time.
Using this return statement
return result+ (result[0] == '0')
;
is a very bad idea because the function in this case can return a pointer that will not store the address of the allocated memory. In this case you will be unable to free the allocated memory using such a pointer.
And at last if passed strings have leading zeroes then you should ignore them. For example "000000" + "1"
should result in "1"
. This makes the function more flexible and general.
And also always check whether memory was allocated successfully.
Here is a demonstratopn program.
#include <stdio.h>
#include <string.h>
char *addBinary( const char *s1, const char *s2 )
{
char *s = NULL;
while ( *s1 == '0' && *( s1 + 1 ) != '\0' ) ++s1;
while ( *s2 == '0' && *( s2 + 1 ) != '\0' ) ++s2;
size_t n1 = strlen( s1 );
size_t n2 = strlen( s2 );
if (n1 == 0)
{
s = malloc( n2 + 1 );
if (s) strcpy( s, s2 );
}
else if (n2 == 0)
{
s = malloc( n1 + 1 );
if (s) strcpy( s, s1 );
}
else
{
size_t max_n = n1 < n2 ? n2 : n1;
int carry = 0;
for (size_t i1 = n1, i2 = n2; ( i1 != 0 && i2 != 0 ) || ( carry && ( i1 != 0 || i2 != 0 ) ); )
{
if ( i1 ) carry += s1[--i1] - '0';
if ( i2 ) carry += s2[--i2] - '0';
carry >>= 1;
}
size_t n = max_n + carry;
s = calloc( n + 1 , sizeof( char ) );
if (s != NULL)
{
carry = 0;
for (size_t i = n, i1 = n1, i2 = n2; i != 0; )
{
if ( i1 ) carry += s1[--i1] - '0';
if ( i2 ) carry += s2[--i2] - '0';
s[--i] = '0' + ( carry & 1 );
carry >>= 1;
}
}
}
return s;
}
int main(void)
{
const char *s1 = "0";
const char *s2 = "0";
char *s = addBinary( s1, s2 );
if ( s ) printf( "\"%s\" + \"%s\" = \"%s\"\n", s1, s2, s );
free( s );
s1 = "11";
s2 = "1";
s = addBinary( s1, s2 );
if ( s ) printf( "\"%s\" + \"%s\" = \"%s\"\n", s1, s2, s );
free( s );
s1 = "1010";
s2 = "1011";
s = addBinary( s1, s2 );
if ( s ) printf( "\"%s\" + \"%s\" = \"%s\"\n", s1, s2, s );
free( s );
s1 = "000000";
s2 = "1";
s = addBinary( s1, s2 );
if ( s ) printf( "\"%s\" + \"%s\" = \"%s\"\n", s1, s2, s );
free( s );
return 0;
}
Its output is
"0" + "0" = "0"
"11" + "1" = "100"
"1010" + "1011" = "10101"
"000000" + "1" = "1"
By the way it seems that you just copied and pasted the solution found by me here
true
can be converted to the integer value1
, andfalse
to the integer value0
.free
.0
or1
, and booleanfalse
andtrue
was glued on later. So you are adding0
or1
to the pointer, to suppress the leading zero.