Segmentation fault while running bubble sort

Question

I am trying to run a bubble sort algorithm which sorts an array in an ascending order but it comes out with segmentation fault in the online compiler and I can't figure out what's going wrong in there because I think that an element in an array should have a size of four, but after I try I couldn't find the solution. Can someone help me to have a look?

#include <iostream>
#include <array>
using namespace std;

void bubble_sort(int arr[]);
void printArray(int arr[]);

int main()
{

    int arr[] = {10, 4, 2, 8, 11, 15};

    bubble_sort(arr);
    printArray(arr);
    // cout<<sizeof(arr)<<endl;

    return 0;
}


void bubble_sort(int arr[])
{
    for (int i = 0; i < sizeof(arr) / 4; i++)
    {
        for (int j = 0; i < ((sizeof(arr) / 4) - 1); j++)
        {
            int temp;
            if (arr[j] > arr[j + 1])
            {
                temp = arr[j];
                arr[j] = arr[j + 1];
                arr[j + 1] = temp;
            }
        }
    }
}

void printArray(int arr[])
{
    for (int i = 0; i < (sizeof(arr) / 4); i++)
    {
        cout << arr[i] << endl;
    }
    cout << "\n";
}

Answer 1

The nested for loop in the bubble_sort has a terminating condition of i < ((sizeof(arr) / 4) - 1). Because the variable i is never incremented in the nested loop, it will loop forever. Try j < ((sizeof(arr) / 4) - 1) instead. This is what causes your segmentation fault.

I'd also recommend taking the size of your array and passing it to your functions as a separate parameter, rather than trying to use sizeof from within the function. As mentioned by 'Some programmer dude', the sizeof function is currently using the size of an *int, not the number of elements in your array. sizeof(arr) in your main function would resolve this.

(This is my first answer on Stack Overflow so forgive me for any formatting errors.)

Answer 2

There are better ways to deal with arrays in C++ e.g.

#include <algorithm>
#include <iostream>

//---------------------------------------------------------------------------------------------------------------------
// 
// using namespace std; <== unlearn to do this. 
//

//---------------------------------------------------------------------------------------------------------------------
// Instead of having to do sizeof(arr)/sizeof(arr[0]) and trying to deal with pointer decay/losing size informatino of the array : 
// Use this syntax int (&arr)[N] to pass an array by reference AND its size N
// (or use std::array<int,N>&)
// I made this a function template so the function will compile for any N
// I also replaced int by std::size_t since that's the common indexing type 
// in collections (and unlike int it cant get to negative indices)

template<std::size_t N>
void bubble_sort(int(&arr)[N])
{
    for (std::size_t i = 0; i < N; i++) // no need to divide by sizeof(int)
    {
        for (std::size_t j = 0; j < N - 1; j++) // <== you had an i here in the comparison in your original code, also a bug
        {
            if (arr[j] > arr[j + 1])
            {
                std::swap(arr[j], arr[j + 1]); // using swap make code explain itself
            }
        }
    }
}

//---------------------------------------------------------------------------------------------------------------------

template<std::size_t N>
void printArray(const int(&arr)[N])     // pass by const content off arr must not be modifiable by print
{
    bool comma = false;
    std::cout << "[";

    // use range based for loop, can't go out of bound.
    for (const auto value : arr)
    {
        if (comma) std::cout << ",";
        std::cout << value;
        comma = true;
    }

    std::cout << "]\n"; // preferably don't use std::endl (it will flush and slow output down)
}

//---------------------------------------------------------------------------------------------------------------------

int main()
{
    int arr[]{ 10, 4, 2, 8, 11, 15 };

    bubble_sort(arr);
    printArray(arr);

    return 0;
}

Answer 3

Modern C++ way to bubble sort:

#include <iostream>
#include <algorithm>
#include <iterator>
#include <ranges>

template <std::ranges::bidirectional_range R>
void bubble_sort(R&& r) {
  while (true) {
      auto it = std::ranges::is_sorted_until(r);
      if (it == std::end(r)) {
          break;
      }
      std::iter_swap(it, std::prev(it));
  }
}

template <std::ranges::forward_range R>
void print(R&& r) {
    for (auto it = std::begin(r); it != std::end(r); ++it) {
        std::cout << *it << ' ';
    }
    std::cout << '\n';
}

int main() {
    int arr[] = {10, 4, 2, 8, 11, 15};

    bubble_sort(arr);
    print(arr);
}

Demo : https://wandbox.org/permlink/Co4k1GA8ozQ9PfDv

Note that:

• You don't need to know the size of array here
• The algorithm isn't restricted to C-style arrays, you can use (doubly) linked lists, vectors, spans, binary trees, or whatever
• The code is much more "descriptive"; you find "where is sorted until", ans "stops if the spot is the end of the range", and if not, "swaps with content at right before that spot"

Answer 4

There are a couple of errors in your code.

1. In the main() function where you declare int arr[], you can get the correct size of the array as follow:

   int array_size = sizeof(arr) / sizeof(arr[0]);

2. However, outside the main() function, and inside the 2 functions bubble_sort(arr[]) and printArray(int arr[]), you will get the wrong size of the array with the code:

   int array_size = sizeof(arr) / sizeof(arr[0]);

   because these 2 functions only sees the input parameter int arr[] as a pointer to int.

3. But, the main reason your code crashes is as follow. In the bubble_sort() function, your second for loop is incorrect and should have been written as follow:

   for (int j = 0; j < (size - i - 1); j++)

====================================

So, I make some small modifications to your original code, and it works as shown below:

#include <iostream>
#include <array>
using namespace std;

void bubble_sort(int arr[], int size);
void printArray(int arr[], int size);

int main()
{

    int arr[] = {10, 4, 2, 8, 11, 15};

    int array_size = sizeof(arr) / sizeof(arr[0]);

    bubble_sort(arr, array_size);
    printArray(arr, array_size);
    // cout<<sizeof(arr)<<endl;

    return 0;
}


void bubble_sort(int arr[], int size)
{
    for (int i = 0; i < size; i++)
    {
        for (int j = 0; j  < size - i - 1; j++)
        {
            int temp;
            if (arr[j] > arr[j + 1])
            {
                temp = arr[j];
                arr[j] = arr[j + 1];
                arr[j + 1] = temp;
            }
        }
    }
}

void printArray(int arr[], int size)
{
    for (int i = 0; i < size ; i++)
    {
        cout << arr[i] << endl;
    }
    cout << "\n";
}

==================

Note: My answer is very similar to what other people have commented before (such as user "Caden Kroonenberg", and "Some programmer dude"). I only want to write the whole correct code for readability and for my own references as well :-).