Segmentation fault implementing quicksort in C

Question

I am trying to execute the Quick Sort code mentioned in the book Data Structures Using C by Reema Thareja.

The issue is that I cannot get the sorted array as output. Instead, when I add the elements in the array, the output screen disappears, and on running the code again I get the following:

I am using the Turbo C++ Version: 3.2 Compiler

#include <stdio.h>
#include <conio.h>
void quicksort(int arr[],int l,int r);
void main()
{
    int arr[10],l,r,i,n;
    printf("Enter Array size: ");
    scanf("%d",&n);
    printf("Enter Elements: ");
    for(i=0;i<n;i++)
    {
        scanf("%d",&arr[i]);
    }
    quicksort(arr,0,n-1);
    printf("The Sorted Array is: ");
    for (i = 0; i < n; i++)
    {
        printf("%d",arr[i]);
    }
    getch();
}
int partition(int arr[],int l,int r)
{
    int left,right,temp,loc,flag;
    loc=left=l;
    right=r;
    flag=0;
    while(flag!=1)
    {
        while((arr[loc]<=arr[right]) && loc!=right)
        {
            right--;
        }
        if(loc==right)
        {
            flag=1;
        }
        else if(arr[loc]>arr[right])
        {
            temp=arr[loc];
            arr[loc]=arr[right];
            arr[right]=temp;
            loc=right;
        }
        if(flag!=1)
        {
            while((arr[loc]>=arr[left])&& (loc!=left))
            {
                left++;
            }
            if(loc==left)
            {
                flag=1;
            }
            else if(arr[loc]<arr[left])
            {
                temp=arr[loc];
                arr[loc]=arr[left];
                arr[left]=temp;
                loc=left;
            }
        }
    }
    return loc;
}
void quicksort(int arr[],int l,int r)
{
    int loc;
    if(l!=r)
    {
        loc=partition(arr,l,r);
        quicksort(arr,l,loc-1);
        quicksort(arr,loc+1,r);
    }
}

Answer 1

Consider this function:

void quicksort(int arr[], int l, int r)
{
    int loc;
    if(l!=r)
    {
        loc=partition(arr,l,r);
        quicksort(arr,l,loc-1);
        quicksort(arr,loc+1,r);
    }
}

Here, our base case is l==r. But if we add a print statement (and some spacing)

void quicksort(int arr[], int l, int r)
{
    if (l != r)
    {
        printf("left: %d, right: %d\n", l, r);
        fflush(stdout);
      //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

        int loc = partition(arr, l, r);
        quicksort(arr, l, loc - 1);
        quicksort(arr, loc + 1, r);
    }
}

and call it with

int arr[] = {4, 8, 1, 4, 5, 1, 8, 5, 7};
int arr_len = 9;
quicksort(arr, 0, arr_len - 1);

we get the following output:

left: 0, right: 8
left: 0, right: 1
left: 0, right: -1
exited, segmentation fault

Whoops: left was 0 and right was -1 and we called partition(arr, 0, -1); which immediately does an out of bounds memory access with the statement arr[right] => arr[-1].

if (l < r) was probably the intent here:

void quicksort(int arr[], int l, int r)
{
    if (l < r)
    {
        int loc = partition(arr, l, r);
        quicksort(arr, l, loc - 1);
        quicksort(arr, loc + 1, r);
    }
}

which enables us to correctly establish the base case for recursion.

---

Beyond this bug, I recommend using space between operators, selecting meaningful names and avoiding unnecessary variables. An example of this is the beginning of the partition function. As written, it's:

int partition(int arr[],int l,int r)
{
    int left,right,temp,loc,flag;
    loc=left=l;
    right=r;
    flag=0;
// ...

But l and r are just being reassigned to left and right and then go unused for the remainder of the function. Writing it like:

int partition(int arr[], int left, int right)
{
    int temp;
    int loc = left;
    int flag = 0;
// ...

is clearer, but even here, we should move tmp (used for swaps) out to a separate function or at least scope it to the block where it's used to avoid stale value bugs. We should #include <stdbool.h> because that's what int flag actually is (or use early returns to eliminate the variable altogether) and improve the loc variable name (it's actually the pivot we're partitioning the integers on):

int partition(int arr[], int left, int right)
{
    int pivot = left;
    bool flag = false; // or remove this completely in favor of `return`
// ...

This is much cleaner.

Also, void main should be int main and use an explicit return 0. I recommend using a modern compiler and development environment and turning on all warnings:

gcc quicksort.c -Wall -Wextra -Werror -O2 -std=c99 -pedantic

This reveals unused variables in main, among other things, and generally reduces pain points.

Also, I recommend reading how to debug small programs which will give you the tools (both conceptual and software) necessary to walk through and reason about programs to localize errors.