Incorrect result when adding binary strings

Question

I am getting an incorrect result when adding two binary strings "11" + "11". I am expecting "110" but the answer I am getting is "010". I did add a special case if the carry is still 1.

char *addBinary(char *str1, char *str2)
{
    int len1 = strlen(str1); //assume both string lengths are same
    char *res = (char *)malloc((len1 + 1) * sizeof(char));
    int i, a, b;
    int carry = 0;
    char result;
    for (i = len1 - 1; i >= 0; i--)
    {
        a = str1[i] - '0';
        b = str2[i] - '0';
        carry = a + b + carry;
        printf("%d %d %d \n", a, b, carry % 2);
        result = carry % 2 + '0'; //convert to character
        carry = carry / 2;
        str1[i] = result; //use the existing string for the result
    }
    if (carry == 0)
    { //if there is no carry just use the existing string and return
        printf("Final without carry %s \n", str1);
        return str1;
    }
    else
    { //there was a carry, so put 1 in the 0th location and copy the string
        res[0] = 1;
        memcpy(res + 1, str1, sizeof(char) * len1);
        printf("Final %s %s %d\n", res, str1,strlen(res));
        return res;
    }
}

int main()
{
    char bin_str1[] = "11";
    char bin_str2[] = "11";
    printf("%s \n",addBinary(bin_str1, bin_str2));
    return 0;
}

Don't forget that you need enough space for the number to grow by one digit and for the null-terminator, so you should allocate len1+2 bytes with malloc. — David Grayson
– David Grayson, Commented Jan 8, 2019 at 19:12
You are playing with a dangerous loop. This for ( i = len1 - 1 ; i >= 0; i--) where a good compiler should warn you that comparison of unsigned expression >= 0 is always true in this/your case. Use i = len -1 and put your code in while ( i > 0 ){ i-- } It should be treated as UB because if is always TRUE you get yourself in an infinite LOOP. — Michi
– Michi, Commented Jan 8, 2019 at 19:43

Osiris · Accepted Answer · 2019-01-08 19:28:45Z

3

You have two main problems:

First when you allocate space for res:

char *res = (char *)malloc((len1 + 1) * sizeof(char));

You need to allocate space for len1+2 (carry and NUL termination), otherwise your memcpy will write out of bounds (you should copy len1+1 elements, otherwise it is not guaranteed that res is NUL terminated). Also note that in the case that you return str1 the memory is leaked.

Second problem occurs when you set the first char of res you do net set it to a printable character:

res[0] = 1;

You should set it to '1' instead.

edited Jan 8, 2019 at 19:28

answered Jan 8, 2019 at 19:23

Osiris

2,83312 silver badges17 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Tim Randall Over a year ago

But what if len2 is longer than len1? So, a third problem: len2 isn't even defined, much less evaluated or used

Osiris Over a year ago

In the original code it is stated //assume both string lengths are same, so this is maybe a given constraint, which i also assumed.

Tim Randall Over a year ago

Ah, you see, like the compiler I ignore comments. Even ones that say //please read this comment which many effectively do. I like to see constraints and assumptions expressed in code

Osiris Over a year ago

@TimRandall I would also prefer to add at least a check strlen(str1)==strlen(str2), rather than trust that the function is called correctly, but I guess it depends ...

Michi · Accepted Answer · 2019-01-08 20:47:11Z

One more thing to think of is, that if you have something like this:

char bin_str1[] = "10";
char bin_str2[] = "11";

Your addBinary() Function does not returns res, but str1.

Now here is the point, if you assign a pointer to your function so that you can release the memory back to the System, you will have an invalid free().

One approach could be like this:

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

char *addBinary(char *str1, char *str2);

int main( void )
{
    char bin_str1[] = "11";
    char bin_str2[] = "11";
    char *ptr = addBinary( bin_str1, bin_str2 );

    if ( ptr )
    {
        printf("%s \n",ptr );
        free( ptr );
    }
}

char *addBinary(char *str1, char *str2)
{
    char *res = calloc( strlen( str1 ) + strlen( str2 ), sizeof( *res ) );
    int a, b, carry = 0;
    size_t i = strlen( str1 ) - 1;

    while ( i > 0 )
    {
        a = str1[i] - '0';
        b = str2[i] - '0';
        carry += a + b;
        printf("%d %d %d \n", a, b,( carry % 2));
        str1[i] = (char)( carry % 2 + '0'); ///convert to character
        carry = carry / 2;
        i--;
    }
    if ( carry == 0 )
    { ///if there is no carry RETURN NULL
        printf("Final without carry %s \n", str1);
        free( res );
        return NULL;
    }
    else
    { //there was a carry, so put 1 in the 0th location and copy the string
        res[0] = '1';
        strcat( res, str1 );
        printf("Final %s %s %zu\n", res, str1,strlen( res ));
        return res;
    }
}

Or, you can drop malloc and use it like this:

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

int addBinary( char *dest, char *const str1, const char *const str2);

int main( void )
{
    char bin_str1[] = "11";
    char bin_str2[] = "11";
    char dest[256] = { 0 };

    if ( addBinary( dest, bin_str1, bin_str2 ) )
    {
        printf("Dest = %s\n", dest );
    }
}

int addBinary( char *dest, char *const str1, const char *const str2)
{
    int a, b, carry = 0;
    size_t i = strlen( str1 ) - 1;

    while ( i > 0 )
    {
        a = str1[i] - '0';
        b = str2[i] - '0';
        carry += a + b;
        str1[i] = (char)( carry % 2 + '0'); ///convert to character
        carry = carry / 2;
        i--;
    }
    if ( carry )
    {
        dest[0] = '1';
        strcat( dest, str1 );
        return 1;
    }
    return 0;
}

I made some changes to your code to reduce it for better reading.

Collectives™ on Stack Overflow

Incorrect result when adding binary strings

2 Answers 2

4 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related