What I am doing wrong with c++ maps? (scary memory errors)

Question

I am new to C++ and I'm having trouble using a map correctly in class that I'm writing. Basically, when I make a new Block object in a test program and call its write method, the program totally shits itself and gives me a double free or corruption error. What is weird is that everything works fine if I uncomment that one line in the Block constructor. I'm guessing I'm missing some basic c++ knowledge but I'm not finding anything useful on google.

Block.h (includes not shown but they are there):

namespace SSDSim{
    class Block{
        public:
            Block(uint block_num);
            ~Block(void);
            void read(uint page_num);
            void write(uint page_num, void *data);
            void erase(void);
        private:
            uint block_num;
            std::map<uint, void *> page_data;
    };
}

Block.cpp:

#include "Block.h"

using namespace std;
using namespace SSDSim;

Block::Block(uint block){
    //page_data[4]= (void *) 0xfeedface;
    block_num= block;
}

...

void Block::write(uint page_num, void *data){
    if (page_data.find(page_num) == page_data.end()){
        page_data[page_num]= data;
    } else{
        cerr<<"Invalid write\n";
        exit(1);
    }
}

test.cpp:

#include <iostream>
#include "../Block.h"

using namespace std;
using namespace SSDSim;

int main(void){
    Block b= Block(0);
    b.write(0, (void *) 0xdeadbeef);
    b.read(0);
//  b.read(4);
    return 0;
}

Just a nitpick, but you should avoid using relative paths such as "../Block.h". These end up causing a lot of trouble when the code gets moved or someone comes along later to read the code. It's usually better to make the path relative from something like the repository root. Most compilers let you add locations to the include search path. With gcc, you can use -Ipath/to/root and then have include files refer to something relative to that location. — Jonathan Sternberg, Jun 21 '10 at 17:28

Justin Ardini · Accepted Answer · 2010-06-21T17:32:44.200

2

Are you sure the error isn't when you call read()? After all, your constructor assigns page[4] to 0xfeedface, an arbitrary place in memory that could point to anything. If you try to then read that location in memory, bad things may happen.

I would try to avoid creating pointers out of arbitrary addresses. Instead, test your program by making some object and passing its address to page[4].

The error could also be in your destructor. Since you are allocating a Block on the stack, its destructor is called when it goes out of scope.

edited Jun 21 '10 at 17:32

answered Jun 21 '10 at 17:21

Justin Ardini

9,768
2
39
46

Yeah, I'm sure. I've tried running the program without any reads. Actually, I've tracked the problem down to a single line in write(): page_data[page_num]= data; Probably should have included that in the original post... oops. Also. I never actually deference or try to free the void pointers that I am storing in the map. – Peter Enns Jun 21 '10 at 17:24
Did you try the same but making a `new Block` instead of using the stack? The error could be in your destructor. – Justin Ardini Jun 21 '10 at 17:25
Hrm. Thanks. The error was in my destructor. I should probably read up on c++ a little more before I keep writing code. Is there a specific reason why a destructor would be screwing up other functions? – Peter Enns Jun 21 '10 at 17:35
@Peter: It's not that the destructor messed up any other functions, its that the destructor got called at the end of main when the `Block` went out of scope, triggering the error. – Justin Ardini Jun 21 '10 at 17:38
It depends what the destructor does. If it deletes an invalid pointer, or one that's already been deleted, then all sorts of badness could happen. You should keep in mind the "rule of three" that sbi mentions, and consider using containers and smart pointers to avoid having to delete objects yourself. – Mike Seymour Jun 21 '10 at 17:40

score 1 · Answer 2 · answered Jun 21 '10 at 17:23

1

It's hard to tell what's wrong, because you show too little code.

However, the Rule Of Three says that, when a class has either a destructor, copy constructor, or an assignment operator, then it very likely needs all three of them.
With that in mind, your SSDSim::Block class looks very suspicious. What does that destructor do that needs to be done but doesn't warrant taking care of copying?

answered Jun 21 '10 at 17:23

sbi

219,715
46
258
445

Yeah. As I mentioned, I'm new to c++ and I'm kind of floundering around at the moment since I have to rush a little on the project. The answer to your question is that it probably doesn't need a destructor and that I need to spend more time familiarizing myself with c++. – Peter Enns Jun 21 '10 at 17:46
1

@Peter: A very good beginner's book is __Accelerated C++__ by Koenig and Moo. It has a steep learning curve, but manages to pack almost all fundamental C++ knowledge into 250 pages. It's main advantage is that it teaches the right idioms for writing good, safe high-level C++. If you like it more detailed, there's __The C++ Primer__ by Lippman et al. Have a look at [The Definitive C++ Book List and Guide](http://stackoverflow.com/questions/388242). – sbi Jun 21 '10 at 17:58
Thanks for the reading suggestions. I will try to get my hands on accelerated c++ asap. – Peter Enns Jun 21 '10 at 18:03
@Peter: Please remember that I had warned you about the steep learning curve. `:)` I don't want to have to take the blame later. – sbi Jun 21 '10 at 18:23

score 0 · Answer 3 · answered Jun 21 '10 at 17:22

You need to show the code for Block::read(), but I think it's a safe bet that you're attempting to access the item returned from getting an element from the map with the key 4 when there is no such element in the map.

(Your commented line adds one, so it works in that case).

What I am doing wrong with c++ maps? (scary memory errors)

3 Answers3