segmentation fault

Write your question here.
I am trying to store the reminders during binary division in a dynamic array. I am getting a segmentation fault in while loop. Unable to figure out why
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27

#include <iostream>

using namespace std;

int main()

{
    int n, i=0, *ptr;
    
    ptr = (int*)malloc(5 * sizeof(int)); 
    cin>>n;
    
    while(n!=1)
    {
        ptr[i]=n%2;
        i++;
    }
    free(ptr);
    for(i=0;i<10;i++)
    {
        printf("%d", ptr[i]);
    }
    
    return 0;
}
Four problems. First, you only allocate room for 5 ints so if the number is above 31 it won't fit into the allocated space. Second, you never reduce n by dividing it by two, so n never changes and your loop that fills the array is infinite. Third, you free the array before you print it. And finally, your print loop goes up to 10 even though you only allocated room for 5 ints.

Another point is that in C++ we use new and delete instead of malloc and free. Also, we use cout instead of printf.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
#include <iostream>
using namespace std;

int main()
{
    int* bits = new int[32];

    int n;
    cin >> n;

    int size = 0;
    for ( ; n > 0; n /= 2)
        bits[size++] = n % 2;

    for (int i = size; i-- > 0; )
        cout << bits[i] << ' ';
    cout << '\n';

    delete[] bits;
}

It should also be noted that there's really no reason to use dynamic allocation here since a statically allocated array would do. And even if you need a dynamic array, in C++ we usually use a std::vector instead.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
#include <iostream>
#include <vector>
using namespace std;

int main()
{
    vector<int> bits;

    int n;
    cin >> n;

    for ( ; n > 0; n /= 2)
        bits.push_back(n % 2);

    for (size_t i = bits.size(); i-- > 0; )
        cout << bits[i] << ' ';
    cout << '\n';
}
Last edited on
You were taught C++ as an extension to C, weren't you? I'm so sorry. :/

dutch did a good job summing up the issues with the existing code, and provided an example of what a more-modern example might look like. I'm going to provide another, since I already typed it up, had to do something, and came back to find I'd been soundly beaten to the punch.

One difference between dutch's example and mine: dutch's prints out the bits in the order of most significant to least significant. Mine prints them from least significant to most significant, as yours would have with some tweaking.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
#include <iostream>
#include <vector>

int main() {
  using namespace std;

  int n;
  if (cin >> n && n > 0) {
    // Read was successful, we can move on.
    // Initialize an std::vector, which is a manager for a dynamic array.
    vector<int> results;
    // The vector starts out empty. We'll be changing that.

    while (n /*!= 0*/) {
      // Add an integer (specifically n%2) onto the end of our vector, growing it.
      results.push_back(n % 2);
      n /= 2;
      // TODO for OP: Look into how this can be done with bitwise operations.
    }
    // Get each element from the vector in order.
    for (int result : results) {
      cout << result;
    }
    // That's it. No free() required.
  } else /* the read wasn't successful, panic */ {
    cerr << "Please enter a valid positive integer!";
    return 1;
  }
}


More information: http://www.cplusplus.com/articles/37Mf92yv/

We could be using a vector (or array) of bools here, and reserve only as much memory as we need, but eh. Learn first, optimize later.

-Albatross
Last edited on
Topic archived. No new replies allowed.