Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Bug in quicksort example (K&R C book)?

This quicksort is supposed to sort "v[left]...v[right] into increasing order"; copied (without comments) from The C Programming Language by K&R (Second Edition):

void qsort(int v[], int left, int right)
{
    int i, last;
    void swap(int v[], int i, int j);

    if (left >= right)
        return;
    swap(v, left, (left + right) / 2);
    last = left;
    for (i = left+1; i <= right; i++)
        if (v[i] < v[left])
            swap(v, ++last, i);
    swap(v, left, last);
    qsort(v, left, last-1);
    qsort(v, last+1, right);
}

I think there's a bug at

(left + right) / 2

Suppose left = INT_MAX - 1 and right = INT_MAX. Wouldn't this result in undefined behavior due to integer overflow?

like image 385
functionptr Avatar asked Jun 26 '11 21:06

functionptr


4 Answers

Yes, you're right. You can use left - (left - right) / 2 to avoid overflows.

like image 173
Artefact2 Avatar answered Nov 01 '22 04:11

Artefact2


You aren't imagining an array with INT_MAX number of elements, are you?

like image 39
Armen Tsirunyan Avatar answered Nov 01 '22 06:11

Armen Tsirunyan


Yes, you're right, although it's possibly just written that way for simplicity -- it's an example after all, not production code.

like image 1
Peter Alexander Avatar answered Nov 01 '22 04:11

Peter Alexander


K&R was always a bit sloppy with their use of unsigned vs signed arguments. Side effect of working with a PDP that had only 16 kilobytes of memory I suppose. That's been fixed a while ago. The current definition of qsort is

void qsort(
   void *base,
   size_t num,
   size_t width,
   int (__cdecl *compare )(const void *, const void *) 
);

Note the use of size_t instead of int. And of course void* base since you don't know what kind of type you are sorting.

like image 1
Hans Passant Avatar answered Nov 01 '22 06:11

Hans Passant