100

I went to a job interview today and was given this interesting question.

Besides the memory leak and the fact there is no virtual dtor, why does this code crash?

#include <iostream>

//besides the obvious mem leak, why does this code crash?

class Shape
{
public:
    virtual void draw() const = 0;
};

class Circle : public Shape
{
public:
    virtual void draw() const { }

    int radius;
};

class Rectangle : public Shape
{
public:
    virtual void draw() const { }

    int height;
    int width;
};

int main()
{
    Shape * shapes = new Rectangle[10];
    for (int i = 0; i < 10; ++i)
        shapes[i].draw();
}
Improve this question
19
  • 1
    Besides the missing semicolon, you mean? (That would be a compile-time error, though, not runtime) Commented Aug 25, 2011 at 19:38
  • Are you sure they were all virtual ? Commented Aug 25, 2011 at 19:39
  • 9
    It Should be Shape ** It's pointing to an array of Rectangles. Then the access should have been shapes[i]->draw(); Commented Aug 25, 2011 at 19:42
  • 2
    @Tony good luck then, keep us informed :) Commented Aug 25, 2011 at 22:37
  • 2
    @AndreyT: The code is correct now (and was also correct originally). The -> was a mistake made by an editor. Commented Aug 25, 2011 at 22:50

4 Answers 4

Reset to default
152

You cannot index like that. You have allocated an array of Rectangles and stored a pointer to the first in shapes. When you do shapes[1] you're dereferencing (shapes + 1). This will not give you a pointer to the next Rectangle, but a pointer to what would be the next Shape in a presumed array of Shape. Of course, this is undefined behaviour. In your case, you're being lucky and getting a crash.

Using a pointer to Rectangle makes the indexing work correctly.

int main()
{
   Rectangle * shapes = new Rectangle[10];
   for (int i = 0; i < 10; ++i) shapes[i].draw();
}

If you want to have different kinds of Shapes in the array and use them polymorphically you need an array of pointers to Shape.

Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

39

As Martinho Fernandes said, the indexing is wrong. If you wanted instead to store an array of Shapes, you would have to do so using an array of Shape *'s, like so:

int main()
{
   Shape ** shapes = new Shape*[10];
   for (int i = 0; i < 10; ++i) shapes[i] = new Rectangle;
   for (int i = 0; i < 10; ++i) shapes[i]->draw();
}

Note that you have to do an extra step of initializing the Rectangle, since initializing the array only sets up the pointers, and not the objects themselves.

Improve this answer

Comments

14

When indexing a pointer, the compiler will add the appropriate amount based on the size of what's located inside the array. So say that sizeof(Shape) = 4 (as it has no member variables). But sizeof(Rectangle) = 12 (exact numbers are likely wrong).

So when you index starting at say... 0x0 for the first element, then when you try to access the 10th element you're trying to go to an invalid address or a location that's not the beginning of the object.

Improve this answer

1 Comment

As a non c++ adept, mentioning of the SizeOf() helped me understand what @R. Martinho was saying in his answer.
0

This answer by CB Bailey is more accurate from the point of view of language specification. It more clearly explains why accessing shapes[i] is undefined behavior. That answer is quoted below with the pointer variable changed to shapes, Shape as the base class and Rectangle as the derived class (instead of p, Base and Derived from that question).

If you look at the expression shapes[1], shapes is a Shape* (Shape is > a completely-defined type) and 1 is an int, so according to ISO/IEC 14882:2003 5.2.1 [expr.sub] this expression is valid and identical to *((shapes)+(1)).

Thus, accessing shapes[i] is not ill-formed. However, it is undefined as explained below.

From 5.7 [expr.add] / 5, when an integer is added to a pointer, the result is only well defined when the pointer points to an element of an array object and the result of the pointer arithmetic also points the an element of that array object or one past the end of the array. shapes, however, does not point to an element of an array object, it points at the base class sub-object of a Derived object. It is the Rectangle object that is an array member, not the Shape sub-object.

Note that under 5.7 / 4, for the purposes of the addition operator, the Shape sub-object can be treated as an array of size one, so technically you can form the address shapes + 1, but as a "one past the last element" pointer, it doesn't point at a Shape object and attempting to read from or write to it will cause undefined behavior.

These paragraphs give a very good idea of how the expression shapes[i] is dealt with.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.