CODEWITHSUNDEEP
c++arrayspointersloops
airplaneman19
airplaneman19

Reputation: 1159

Why isn't this code printing my array?

I have an integer array sorting program here but I have a problem: whenever I run the program I sometimes get a "The stack around the variable 'numbers' was corrupted" message and sometimes it just repeatedly prints out the number 8. Here's my code (compiled in Visual C++ 2010):

#include <iostream>
#include <cstdlib>
using std::cout;
using std::endl;

void swap(int *x, int *y)
{
    int tmp=0;
    tmp = *x;
    *x  = *y;
    *y  = tmp;
    tmp = 0;
}

int main()
{
    int numbers[13] = {8,16,23,487,2,301,48,0,13,10,644,12};

    int size = sizeof(numbers) / sizeof(int);

    //sort

    int i = 0;
    int* a = &numbers[0];
    int* b = &numbers[1];


    while(i < size){

        if(*a > *b){
            swap(a, b);
        }

        *a++;
        *b++;
         i++;
    }

    //Print our results
    int loopIterator = 0;
    int numToPrint = 0;
    while(loopIterator < size){
        cout << numbers[numToPrint] << endl;
        loopIterator++;
    }


    system("PAUSE");

}

Upvotes: 1

Views: 184

Answers (5)

Void - Othman
Void - Othman

Reputation: 3481

I assume you are implementing the array sort as an exercise. This doesn't really answer your question but I thought I'd post for reference, regardless. Here's one way to achieve the desired result using the STL:

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

int main()
{
  int numbers[] = { 8, 16, 23, 487, 2, 301, 48, 0, 13, 10, 644, 12 };
  size_t const size = sizeof(numbers) / sizeof(numbers[0]);

  int * const begin = numbers;
  int * const end   = numbers + size;

  std::sort(begin, end);
  std::copy(begin, end, std::ostream_iterator<int>(std::cout, "\n"));
}

Upvotes: 2

KevinDTimm
KevinDTimm

Reputation: 14376

This sort won't work as you only run through your list of numbers once, so it will swap adjacent items but it won't sort the list (it's kind of half an implementation of a bubble sort)

Upvotes: 0

Karoly S
Karoly S

Reputation: 3268

Well one thing that immediately jumps out at me is that you are never incrementing numToPrint so it will print out numbers[0], size number of times.

I would rewrite your printing section to 

for (int i = 0; i < size; i++)
    cout << numbers[i] << endl;

With that you can get rid of the print results section of your code, as the above is a cleaner way of doing the same thing.

The error message you are getting is probably because you are writing to parts of memory that you shouldn't be touching. This is probably a result of using "sizeof" incorrectly. It returns the number of elements in numbers, not the memory size. Would recommend checking the comments on the actual question for the correct solution to your second issue.

Upvotes: 0

Ben Voigt
Ben Voigt

Reputation: 283823

I'm pretty sure you have an operator precedence problem here:

*b++;

In fact, the compiler should have warned you about an operator with no side-effects (the *).

Besides that, pointer b will go off the end of the array, since it starts at element 1 and is advanced size times, it will end up pointing to numbers[size+1]. If the compiler optimizes away the useless dereference, that won't be a problem, but on the previous pass you call swap(numbers+size-1, numbers+size) and that writes off the end of the array, causing the stack corruption you detected.

Upvotes: 0

Jason
Jason

Reputation: 32538

First, you never increment numToPrint, therefore you are never going to print more than the value of numbers[0]. At the very least change your code to:

while(loopIterator < size){
    cout << numbers[numToPrint++] << endl;
    loopIterator++;
}

Secondly, since your while loop uses the test i < size, you are, on the last iteration of the loop, going to be accessing memory outside of numbers for your b pointer, and possibly swapping that value into the last slot of numbers (i.e., where a is pointing to). You want to change your test to i < (size - 1) to avoid that scenario. For instance, if at i == 0 you have a = &numbers[0] and b = &numbers[1], then by the time that i == 12, you are going to end up with a = &numbers[12] and b = &numbers[13] ... the value that b is pointing to in this instance is past the end of the array. Depending on how your compiler has setup the stack, and the way you've allocated numbers on the stack, this could actually play some havoc with your program should you end up with b pointing into the activation record data-structures for your main() function, and in-turn corrupting it.

Upvotes: 2

Related Questions