C++ STD Vector push_back doesn't seem to work

Question

I'm making a game with SDL that used libconfig to read some settings from a file. The problem is that I made a class called ClipList that contains a std::vector<SDL_Rect> to store the settings but when trying to add SDL_Rect objects to the vector, for some reason push_back does nothing and I end up with an empty vector.

This is the class:

class ClipList
{
    public:
        ClipList();
        ClipList(int);
        virtual ~ClipList();
        void addClip(int,int,int,int);
        void getClip(int,SDL_Rect*);
        int getLength();
    protected:
    private:
    std::vector<SDL_Rect> clips;
};
ClipList::ClipList(int l)
{
    clips.reserve(l);
}

void ClipList::addClip(int x,int y,int w,int h){
    SDL_Rect rect;
    rect.x = x;
    rect.y = y;
    rect.w = w;
    rect.h = h;
    clips.push_back(rect);
}

void ClipList::getClip(int i,SDL_Rect* rect){
rect = &(clips.at(i));
}

int ClipList::getLength(){
    return clips.size();
}

And this is the function where I initialize the ClipList object. This function gets called from main.

void set_clips(Config* placlips,ClipList* clips, ClipList* flipclips){
    const Setting& root = placlips->getRoot();
    int x,y,w,h;
    try{
        Setting& clipsett = root["clips"];
        int cliplen = clipsett.getLength();
        clips = new ClipList(cliplen);
        flipclips = new ClipList(cliplen);
        for(int i=0;i<cliplen;i++){
            const Setting& c = clipsett[i];
            if(!(c.lookupValue("x",x)&&c.lookupValue("y",y)&&c.lookupValue("w",w)&&c.lookupValue("h",h))){
                continue;
            }
            clips->addClip(x,y,w,h);
        }
    }catch(const SettingNotFoundException &nfex){
        cerr << "Setting not found at" << nfex.getPath() << endl;
    }
}

Regardless of whether the ClipList objects get initialized in main or set_clips, clips.push_back(rect) doesn't work. The capacity of the vector changes but no object gets stored so I end up with a segfault if I try to do anything else with the vector, even checking if the vector is empty or not.

score 1 · Accepted Answer · answered Apr 13 '14 at 03:49

1

I am going to guess, the signature of the function

void set_clips(Config* placlips,ClipList* clips, ClipList* flipclips);

is the culprit. You are allocating memory for clips and flipclips in this function but since the pointers are passed by value, the calling function does not see the allocated memory.

If you change the function signature to:

void set_clips(Config* placlips, ClipList*& clips, ClipList*& flipclips);

your problems should go away.

answered Apr 13 '14 at 03:49

R Sahu

204,454
14
159
270

Yes, that was the problem, though passing the pointers by reference got the code to compile the `SDL_Rect` objects still didn't get stored in memory. I got around the problem by initializing the ClipList objects in `main`, and passing the ClipList objects by reference instead of the pointers. – Magnus Apr 13 '14 at 22:56

janm · Answer 2 · 2014-04-13T22:43:35.300

clips.push_back(rect) is working fine. Your set_clips function allocates new ClipList instances but does not pass those pointers back to the caller. The caller is probably attempting to use a garbage pointer as an initialise instance and that is why you are getting a segfault.

You need to pass the created objects back. You should use something like std::shared_ptr<> to do that instead of bare pointers.

Update on how to do this without using std::shared_ptr<>:

You need to keep track of ownership and deal with exceptions. In terms of the actual passing, the rule I use (originally from Lakos in "Large Scale C++ Software Design") is that parameters that are return values (as you are attempting to use them) are pointers, and read-only parameters are by value or const-reference. Return values come first.

So, your set_clips function should look like this:

void set_clips(ClipList** clips, ClipList** flip_clips, Config const& placlips)

When you call set_clips you pass a pointer to each pointer that will receive the allocated value, and pass a const-reference to the placlips object that is not modified by the function.

You would all it something like this:

ClipList* clips = 0;
ClipList* flip_clips = 0;
set_clips(&clips, &flip_flips, placlips);
// ... then do whatever comes next.

But combining those rules with std::shared_ptr<> or boost::shared_ptr<> is better and the "modern C++" style.

How can I do that without shared_ptr? – Magnus Apr 13 '14 at 22:26 — Magnus, Apr 13 '14 at 22:26

C++ STD Vector push_back doesn't seem to work

2 Answers2