C program for changing binary to signed integer doesn't work properly

Question

I have a C program that I made that is supposed to convert binary into a signed integer, but it does not give the correct result.

Here is the function of the code that converts binary numbers to signed integers:

#include <math.h>
#include <stdio.h>
#include <string.h>
int bin_to_sint(char *bin) {
 int sint = 0;
 int power;
 for (int i = 0; i < strlen(bin); i++) {
 power = strlen(bin) - (i + 1);
 if (bin[0] == '1') {
 sint += -pow(2, power);
 printf("%d\n", sint);
 continue;
 }
 if (bin[i] == '1') {
 sint += pow(2, power);
 }
 printf("%d\n", sint);
 }
 return sint;
}
int main(void)
{
 printf("%d\n", bin_to_sint("1110"));
}

Here is the result that prints out. For this, I entered the sample number 1110. Instead of giving -2 as the result (as shown in the book Computer Systems: A Programmer's Perspective), it gives out -15.

Signed int: -15

Can someone figure out the problem?

• ti7: your check of bin[0] each loop iteration is flawed - you almost-certainly want to discover the sign once outside the loop
• Sebastian Llaurador: Your suggestion was able to work!
• ti7: excellent - if you have a complete Answer now, consider posting it too! the other current Answers are technically excellent and show some of the depth needed to be precise in C even for something relatively simple, but an Answer in terms of your learning (and perhaps a class) are often very valuable to complete the steps in-between
• John Bollinger: Don't use pow() where there are better alternatives -- which is almost always the case when you have mathematical reason to expect the result to be an exact integer, representable via one of the built-in integer types. Integer powers of 2, in particular, are often better computed by left-shifting 1 (e.g. 1 << power).
• John Bollinger: Why shouldn't 1110 yield 14 decimal? Note well that twos' complement representations are referenced to a specific number of total bits.

Answer 1

Mixing floating point and integer arithmetics is always risky and in this case incorrect and unnecessary.

The main problem in your code is you test for bin[0] == '1' at each iteration and always subtract the ith bit regardless of its value. You should do this only if bin[i] == '1'

Here is a modified version that uses integer arithmetic:

#include <limits.h>
#include <stddef.h>
int bin_to_sint(const char *s) {
 union { unsigned val; int ival; } u = { 0 };
 size_t i;
 for (i = 0; s[i] >= '0' && s[i] <= '1'; i++) {
 u.val = (u.val << 1) + (s[i] - '0');
 }
 if (s[0] == '1' && i < sizeof(u.val) * CHAR_BIT) {
 /* sign extend the value */
 u.val -= 1U << i;
 }
 /* reinterpret the bits as an int */
 return u.ival;
}

• chqrlie: @chux: interesting link! I hate this new commenting interface, it is 100% counterproductive.
• chux: Agreed.........

Answer 2

What Is Wrong With The Function

1. Incorrect two's-complement handling

• Checking bin[0] == '1' inside the loop and subtracting 2^power on every iteration accumulates -(2^L - 1), which is wrong.
• Correct approach: parse all bits into an unsigned value, then if the top bit is 1, subtract 2^L exactly once at the end.

2. Recomputing strlen in the loop

• strlen(bin) is called twice per iteration, making the loop O(n^2).
• You do not need the length at all; count characters as you scan.

3. Using pow for integer arithmetic

• pow returns double. Converting back to int risks precision loss and is slower.
• Use integer shifts for powers of two.

4. Skips lower-bit contributions when negative

• The continue under the bin[0] == '1' branch prevents adding contributions from later bits.

5. Interface and type concerns

• Parameter should be const char * since the function does not modify the string.
• Mixing int with size_t from strlen is error prone.
• Printing inside the converter adds side effects; keep conversion and I/O separate.

Better Guidance

• Never use floating point functions like pow for integer arithmetic.
• Do not call strlen in each iteration; you do not need it at all.
• Parse by shifting and or-ing bits into an unsigned accumulator as you scan.
• After the scan, if the first bit was 1, subtract 2^L once to get the two's-complement value.
• Keep the converter pure: return the value and do any printing outside.

Here is a cleaner version:

Below are functions that perform the conversion (with minimal or no error checks).

Assumptions: the input string is a two's-complement value, and its bit width equals the number of characters in the string. For example, "10101" is a 5-bit two's-complement number.

#include <stdio.h>
#include <stdint.h>
int binstr_to_int_tc(const char *s)
{
 int result = 0;
 uint64_t u = 0;
 size_t n = 0;
 char msb = '0';
 if (s)
 {
 msb = s[0];
 while (s[n] != '0円')
 {
 char c = s[n];
 u = (u << 1) | (uint64_t)(c - '0'); /* assumes '0'/'1' only */
 ++n;
 }
 if (msb == '1' && n < 62)
 {
 u -= ((uint64_t)1 << n); /* subtract 2^n in unsigned domain */
 }
 result = (int)u; /* cast down to int width */
 }
 return result;
}
int main(void)
{
 const char *a = "0111"; /* +7 in 4 bits */
 const char *b = "1110"; /* -2 in 4 bits */
 const char *c = "1000"; /* -8 in 4 bits */
 const char *d = "11111111"; /* -1 in 8 bits */
 const char *e = "11111110"; /* -2 in 4 bits */
 const char *f = "00000111"; /* 7 in 4 bits */
 printf("%s => %d\n", a, binstr_to_int_tc(a));
 printf("%s => %d\n", b, binstr_to_int_tc(b));
 printf("%s => %d\n", c, binstr_to_int_tc(c));
 printf("%s => %d\n", c, binstr_to_int_tc(d));
 printf("%s => %d\n", c, binstr_to_int_tc(e));
 printf("%s => %d\n", c, binstr_to_int_tc(f));
 return 0;
}

https://godbolt.org/z/vEs5xc9Wf

• Chris: 100% opinion, but I would suggest your if (s) { ... } conditional introduces extraneous nesting. An early exit on if (!s) would make this easier to read to my eyes.
• 0___________: I disagree, as I dislike many return points.
• Chris: Does the check for n == 0 need to be inside the loop and evaluated on every iteration?
• 0___________: No it does not
• chqrlie: I'm afraid this approach does not work for numbers with 64 or more bits (eg: 1111111111111111111111111111111111111111111111111111111111111111) as ((uint64_t)1 << n) has undefined behavior and likely produces 1 instead of 0 for n == 64.
• 0___________: yes, << 64 is UB — shocking, I know. But since this helper returns an int, the bit width is already capped at 8*sizeof(int), so the "64-bit shift" cliff is not exactly a plot twist. Add a quick guard (or use a wider accumulator) and we are golden.
• chqrlie: I understand the return value limits the conversion, but why not write if (msb == '1' && n < 64) and avoid UB and surprising result?
• chux: As int is allowed to be 64-bit and so uint64_t is not certainly wider than int, I'd recommend using unsigned instead of uint64_t and use alternative logic to handle edge/wide cases.
• 0___________: Also look at the return type