C++ read memory faild

Question

I have 2 classes:

class b_class
{
public:
    int time_viewed;
    int parent_course;

    b_class()=default;

    b_class(int parent_course):time_viewed(0), parent_course(parent_course){
    }
};

and:

class b_course
{
public:

    int course_id;
    int num_of_classes;
    b_class **classes;

    b_course(int course_id,int num_of_classes);
};

In the latter I wrote the following code:

b_course::b_course(int course_id,int num_of_classes) {
    this->course_id=course_id;
    this->num_of_classes=num_of_classes;
    classes=new b_class*[num_of_classes*sizeof(b_class*)];
    for (int i=0;i<num_of_classes;i++)
    {
        classes[i]->time_viewed=0;
        //classes[i]->parent_course=course_id;
    }
}

But I'm getting an error because I am trying to access some memory which shouldn't be accessed.

read memory from 0x7000000000000000 failed (0 of 4 bytes read)

anyone know what is the reason for this?

classes is an array of pointers to b_class

`classes=new b_class*[num_of_classes*sizeof(b_class*)];` is suspicious. I'd expect `classes=new b_class[num_of_classes];` and then more code to "new" each array element. — chux - Reinstate Monica, Nov 28 '20 at 21:50
new int[7] will allocate an array of 7 int elements not an array of 7 pointers to ints — , Nov 28 '20 at 21:57
Please don't delete the code from your question, it makes the answers incomprehensible. — Nate Eldredge, Nov 30 '20 at 00:09
According to [this help page](https://stackoverflow.com/help/what-to-do-instead-of-deleting-question), you are not permitted to delete your question, because this invalidates the existing answer. Your original question has therefore been restored. Please don't delete it again. — Andreas Wenzel, Dec 01 '20 at 19:51
@AndreasWenzel my partner refuses to let me share this code, it's not my decision :( — , Dec 01 '20 at 22:21
Please don't make more work for other people by vandalizing your posts. By posting on the Stack Exchange network, you've granted a non-revocable right, under the CC BY-SA 4.0 license, for Stack Exchange to distribute that content (i.e. regardless of your future choices). By Stack Exchange policy, the non-vandalized version of the post is the one which is distributed. Thus, any vandalism will be reverted. — cigien, Dec 01 '20 at 22:23

Remy Lebeau · Accepted Answer · 2020-11-28T22:02:10.143

2

You are allocating an array of pointers that don’t point at any valid objects. And you are allocating more space for the array then you actually need.

b_course::b_course(int course_id,int num_of_classes) {
    this->course_id = course_id;
    this->num_of_classes = num_of_classes;
    classes = new b_class*[num_of_classes]; // <- get rid of sizeof() here
    for (int i = 0; i < num_of_classes; i++)
    {
        classes[i] = new b_class(/*course_id*/); // <- add this
        classes[i]->time_viewed = 0;
    }
}

That being said, a better option is to allocate an array of objects instead of an array of pointers, eg:

class b_course
{
public:

    int course_id;
    int num_of_classes;
    b_class *classes;

    b_course(int course_id, int num_of_classes);
};

b_course::b_course(int course_id, int num_of_classes) {
    this->course_id = course_id;
    this->num_of_classes = num_of_classes;
    classes = new b_class[num_of_classes];
    for (int i = 0; i < num_of_classes; i++)
    {
        classes[i].time_viewed = 0;
        //classes[i].parent_course = course_id;
    }
}

Either way, since you are allocating objects dynamically, be sure to follow the Rule of 3/5/0 by also implementing a destructor to free the objects and array, and a copy constructor and copy assignment operator to make copies of the array and objects from one b_course to another.

The best way to handle all of this is to use a std::vector instead of new[], and let the compile handle the details for you, eg:

#include <vector>

class b_course
{
public:

    int course_id;
    std::vector<b_class> classes;

    b_course(int course_id,int num_of_classes);
};

b_course::b_course(int course_id,int num_of_classes) {
    this->course_id=course_id;
    this->classes.resize(num_of_classes);
    for (int i=0;i<num_of_classes;i++)
    {
        classes[i].time_viewed=0;
        //classes[i].parent_course=course_id;
    }
}

edited Nov 28 '20 at 22:02

answered Nov 28 '20 at 21:48

Remy Lebeau

555,201
31
458
770

Please not I want it to be an array of pointers and not to change that – Nov 28 '20 at 21:53
Sorry I didn't mention that but I am leaning C++ and don't want to use vector, again this doesn't answer my original question of an array of pointers and not objects – Nov 28 '20 at 21:59
1

@john note that the question you asked is "what is the reason for this?" The correct answer for that is in the first two sentences. It does answer your question. The rest of the answer is a nice "how to fix it?" add on. If you absolutely need an array of pointers you should say that in the question and perhaps explain why you need that – 463035818_is_not_an_ai Nov 28 '20 at 21:59
@john I have updated my answer to address your question. However, what you are asking for is a really bad idea, and should be avoided. – Remy Lebeau Nov 28 '20 at 22:00
2

@john also note that if you want to learn C++ then this is a reason to use `std::vector` rather than a reason to not use it – 463035818_is_not_an_ai Nov 28 '20 at 22:00
Thanks, why you removed sizeof()? if the number was 7 then you are allocating 7 spaces in memory but sizeof pointer is not 1 – Nov 28 '20 at 22:04
3

I agree with @idclev463035818. Too many teachers and textbooks approach C++ as if it were just C with OOP added on, which is not true. C and C++ have evolved into very different languages. There are plenty of [good C++ books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) that teach C++ the right way. – Remy Lebeau Nov 28 '20 at 22:05
@john When you create the array, you are *creating* pointers since the array is an array of pointers. If a pointer is 4 bytes and you want to create 10 of them, the implementation isn't creating 40 of *anything*. So why would you pass it a number it cannot use? When you use `new`, you are actually creating new objects. Pointers exist that did not exist before and the implementation needs to know how many to make exist. – David Schwartz Nov 28 '20 at 22:06
@john `new` and `new[]` in C++ are not the same as `malloc()` in C. `new`/`new[]` take the size of the requested type into account for you, so you don’t need to use `sizeof()` manually. You ask `new[]` for how many *elements* you want for the array, not for how many *bytes* it will need. This is fully covered in decent C++ book. – Remy Lebeau Nov 28 '20 at 22:06
2

@john suggest you to watch this: [Stop teaching C](https://www.youtube.com/watch?v=YnWhqhNdYyk), rather provocative title, but the talk nails the problem nicely – 463035818_is_not_an_ai Nov 28 '20 at 22:06

C++ read memory faild

1 Answers1