Sign in to follow this  
ukdeveloper

Access violation on vector push_back()

Recommended Posts

I've been getting somewhat bizarre behaviour resulting in a crash, when I attempt to push_back a pointer to an object into a vector. I've done this before without any problems so I really can't see what's happening here. Essentially, I've got a class called CCard, which looks like this:
#pragma once
#include <SDL/SDL.h>
#include <iostream>
#include <string>

class CCard
{
public:
	CCard(void); 
	~CCard(void);
	CCard(const CCard& cpyCard);
	std::string returnSuit();
	int returnRank();
	void setSuit(std::string suit);
	void setRank(int rank);
	//SDL_Surface* cardGraphic;

private:
	std::string cardSuit;
	int cardRank;
};



and its constructor:

CCard::CCard(void)
{
	// Set rank, suit and graphic
	// Decide on parameters once I've figured out the graphics part

	//this->cardGraphic = NULL;

	std::cout<<"CCard constructor running...";
	cardRank = 1;
	cardSuit = "testCard";
	std::cout<<"done."<<std::endl;
}



so you'll notice that nothing is uninitialised inside the constructor. The constructor completes running successfully. Here's where I'm using it:
tempCard = new CCard();

				std::cout<<"Attempting vector pushback...";
				// Crash occurs w/ access violation when pushing back tempCard!
				cards.push_back(tempCard);
				std::cout<<"done."<<std::endl;



^^ This is inside the inner part of a nested for loop tempCard is declared further up in the code and is actually instantiated here. This is purely for testing purposes and the CCard object doesn't do anything as of yet. cards is a vector of CCard pointers and if I comment out the push_back then it doesn't crash, but it's then doing nothing at all except creating the object over and over and rewriting it on each pass of the loop. When cards.push_back(tempCard) runs, it crashes with
Unhandled exception at 0x00414546 in BlackJack_SDL.exe: 0xC0000005: Access violation reading location 0xccccccd4.
and this points to size_type size() inside the vector header itself. When tracing through with the debugger, I notice the following: clicky. and when I go to the watch again I notice that the cardSuit variable has doubled in length and the second half is null. I'm guessing that's not a nice thing to have even if it isn't the problem. I've been stuck for days but, as always, I'm sure it's screamingly obvious. I can't see it, the only thing I can think is something to do with that strange doubling in length of the variable. I'll reiterate: I've done this type of thing before and it's never had these problems, except in the other scenario where it worked I used a dynamically created pointer array instead of a vector. Any help would be greatly appreciated. TIA, ukd.

Share this post


Link to post
Share on other sites
Debuggers and debug builds often use special values for special values. For example, your error is at the address 0xccccccd4, which is close to the magic code 0xcccccccc, which appears to indicate an uninitialised local variable. Now, std::vector is always initialised unless you haven't initialised the containing object. Therefore it is my guess that the std::vector is a member of a class. You are calling the push_back() inside a member function on an uninitialised pointer to that class

This is what is probably happening:

class Card { };

class Example
{
public:
void add( const Card & );
private:
std::vector<Card> cards;
};

void foo()
{
Example *example;
example->add(Card());
}






Now, other points:

  • Don't write destructors if they would be empty.

  • Don't write copy contructors for classes when it isn't required. In this case it isn't because your copy would be the same as the compilers auto-generated version.

  • Pass and return strings by const reference.

  • Prefer to initialise objects using a constructor rather than set functions.

  • Real cards don't change their suit nor their rank. Your card objects shouldn't either.

  • Don't store pointers in a container unless polymorphism is required.

  • If polymorphism is required, use smart pointers or smart containers.

Share this post


Link to post
Share on other sites
Post the code containing the full context of "cards" -- that is, from it's declaration to the point where the crash occurs.

rip-off beat me to a more in-depth description. But for some more detail about the special values like 0xCCCCCCCC, et cetera, this is a handy reference.

Share this post


Link to post
Share on other sites
When cards.push_back(tempCard) runs, it crashes with

Unhandled exception at 0x00414546 in BlackJack_SDL.exe: 0xC0000005: Access violation reading location 0xccccccd4.


I assume cards is a member of a class?
0xCCCCCCCC is a debug fill pattern that means a variable hasn't been initialized [clicky].

Since std::vector is self-initializing, this almost certainly means you called the function that's crashing on an invalid pointer, like this:

WhateverContainsCards *ptr;
ptr->WhateverAddsACard();


Your program doesn't crash until you try to use the first member of WhateverContainsCards, when it first uses the implicit "this" pointer which was passed to it.

If you need more help, post more code, especially the code calling the function you're crashing in.

HTH,
-Mike

Share this post


Link to post
Share on other sites
Umm... yes, you were all correct. I feel so stupid now.

The game consists of a driver class, and numerous states which I'm using for the first time, incidentally. The driver class contains an instance of the CPack class as one of its fields, but I didn't initialise the CPack object inside the driver class's constructor. The vector is indeed inside a class, inside the formerly-uninitialised CPack class to be precise, and so the vector was uninitialised when I tried to use it.

I've done that and it now works properly. Before you guys replied, I would never have even thought to look for something as basic as that :-/

Thanks to all of you. I've added "I won't be a dumbass" to my unwritten New Years' Resolutions list [grin].

Share this post


Link to post
Share on other sites
Quote:
Original post by ukdeveloper
Umm... yes, you were all correct. I feel so stupid now.

Don't worry. This is a fairly common problem that bites a lot of people and isn't at all obvious unless you've experienced it before. Plus, you gave enough information -- namely, the error message and the problematic line of code in this case -- that you had people tripping over each other to answer your question.

I'd say most of the people with this kind of problem fail to accomplish that in their initial post, which usually results in us poking and prodding for code, context, error messages... we like it when we don't have to do all that.

Quote:
I've added "I won't be a dumbass" to my unwritten New Years' Resolutions list [grin].

Nah, make it "Keep asking questions the smart way". Think positive :).

Share this post


Link to post
Share on other sites
Quote:
Original post by rip-off
Now, other points:

  • Don't write destructors if they would be empty.

  • Don't write copy contructors for classes when it isn't required. In this case it isn't because your copy would be the same as the compilers auto-generated version.

  • Pass and return strings by const reference.

  • Prefer to initialise objects using a constructor rather than set functions.

  • Real cards don't change their suit nor their rank. Your card objects shouldn't either.

  • Don't store pointers in a container unless polymorphism is required.

  • If polymorphism is required, use smart pointers or smart containers.




  • In constructors, use initialization lists.

  • Don't write (void) for function signatures unless you need to interact with existing C code. Write().

  • Using 'return' as a prefix for function names is strange; of course the function returns something.

  • Do you eat FApples? No? Then why would you code CCards? The fact that the apple is a fruit is never something that you need to think about whenever you are doing any of the things you can do with an apple. You only think about it when you are talking about apples. Similarly, the fact that your card is represented by a class is totally irrelevant to using it, and only matters in terms of implmentation - at which point the language keyword 'class' is already staring you in the face.

  • Similarly: does your wallet contain walletMoney? No? Then why would your card have a cardSuit and cardRank?

  • Don't rely on #pragma once. #pragmas are compiler-specific. Certainly you can include it, but don't rely on it.

  • Raw accessor/mutator pairs are basically useless busywork. Of course, since we shouldn't have mutators anyway, this is irrelevant this time.

  • Don't include things in headers that are not going to be used by the header. Let everything include the things that *it* needs. Don't depend on other things, because then you, well, create dependencies.




  • #ifndef CARD_H
    #define CARD_H

    #pragma once
    #include <string>

    class Card {
    std::string suit_;
    int rank_;

    public:
    Card(int rank, const std::string& suit): rank_(rank), suit_(suit) {}
    std::string suit() { return suit_; }
    int rank() { return rank_; }
    };
    #endif

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this