1

I am trying to allocate memory to structure dynamically but i am getting error while allocation the memory dynamically

#include <string>

using namespace std;

#define MAX_GRADES 5

typedef struct _Student
{
    string id;
    string name;
    double grades[MAX_GRADES];
    double average;
}Student;

Student *update_list;

int main()
{
    Student *n = (*Student) malloc(sizeof(n));
    return 0;
}
main.cpp: In function ‘int main()’:
main.cpp:26:24: error: expected primary-expression before ‘)’ token
  Student *n = (*Student) malloc(sizeof(Student));
                        ^
Mat
  • 202,337
  • 40
  • 393
  • 406
Rajeev
  • 147
  • 1
  • 1
  • 17
  • 1
    Important note: `std::string` is a fairly complex class. It is only valid if at least one of its many constructors successfully initializes the class. `malloc` is a C function. C doesn't know what a constructor is, so no `string` constructor is run. This leaves `id` and `name` ticking time bombs waiting to blow your program up. Do not `malloc` a C++ class unless you know the class is a [POD type](https://stackoverflow.com/questions/146452/what-are-pod-types-in-c) or will be using [placement `new`](https://stackoverflow.com/questions/222557/what-uses-are-there-for-placement-new) before use. – user4581301 Nov 14 '18 at 06:46

5 Answers5

4

TL;DR version

Use

Student *n = new Student;

instead of

Student *n = (*Student) malloc(sizeof(n));

Explanation

This is a typo

Student *n = (*Student) malloc(sizeof(n));
              ^ wrong side.

You want

Student *n = (Student*) malloc(sizeof(n));
                     ^ needs to be here.

This will compile. Normally I would leave a comment and vote to close but there are two things wrong with this solution that you'll find at runtime.

  1. n is a pointer. sizeof(n) will give the size of a pointer which is guaranteed to be shorter than a string (unless the string implementation is incredibly awesome and doing things I can't even imagine). You would want sizeof(*n) to get the size of a Student if the next point didn't render this point useless.
  2. std::string is a fairly complex class. It is only valid if at least one of its many constructors successfully initializes the class. malloc is a C function. C doesn't know what a constructor is, so no string constructor is run. This leaves id and name ticking time bombs waiting to blow your program up. Do not malloc a C++ class unless you know the class is a POD type or will be using placement new before use.

Use new here or do not dynamically allocate at all.

user4581301
  • 33,082
  • 7
  • 33
  • 54
3

Student *n = (*Student) malloc(sizeof(n));

(*Student) is not a type, you want (Student*). Also, sizeof n gives the size of a pointer on your system, not the size needed for one or more Students. Correct way:

Student *n = (Student*) malloc(sizeof(*n));

However, as pointed out in the answer of user4581301, your Student is not POD so you have to use new to ensure constructors are being called:

Student *n = new Student;

or, if you want more than one Student

Student *n = new Student[42];

Please remember that all memory allocated using new must be deallocated using delete and all memory allocated using new[] with delete[].

But the use of raw owning pointers is considered bad practice and should be avoided in favour of smart pointers like std::unique_ptr<> and std::shared_ptr<> or containers:

The C++-way would be using std::vector<Student>.
Also, there is no reason to use a typedef for a structure in C++.

Swordfish
  • 12,971
  • 3
  • 21
  • 43
  • 1
    Actually it is not correct to use ```malloc``` in allocating memory for an object like in the question, which contains a ```string```. See the discussion https://stackoverflow.com/questions/40434360/c-memory-error-when-using-malloc-realloc-free-on-stdstring Use ```new``` instead – francesco Nov 14 '18 at 08:44
  • @francesco Oh, i missed the `string`s. Assumed it was a POD :/ – Swordfish Nov 14 '18 at 08:46
1

There are three mistakes in your code:

  • The cast right before malloc should be (Student *): this is the correct type of n, i.e., a pointer to Student.
  • The size to allocate is sizeof(Student): sizeof(n) would give you the size of a pointer, most probably too small for Student.
  • Most important: you should never use malloc to allocate memory for a struct which contains a complex object like a string. You must use instead new, see the discussion here.

A correct code would be

#include <string>

using namespace std;

#define MAX_GRADES 5

struct Student
{
    string id;
    string name;
    double grades[MAX_GRADES];
    double average;
};

Student *update_list;

int main()
{
    Student *n = new Student;

    delete n;
    return 0;
}

I have added a delete n at the end: it is not necessarily to run the code correctly, but it is a good practice to not forget to delete the allocated memory.

Even better, rather than using a #define to fix the size of the array grades you should use a template parameter: in this way MAX_GRADES does not propagate in all the following code. Finally, in general it is better to avoid using namespace std; in the header part. This is because, if you ever split the declaration of Student in a separate header file, than every code file which include such a header will "inherit" the using namespace std; which could, possibly, collide with other included headers.

In summary an even better version of your code is

#include <string>

template <unsigned int MAX_GRADES>
struct Student
{
    std::string id;
    std::string name;
    double grades[MAX_GRADES];
    double average;
};

Student<5> *update_list;

int main()
{
    Student<5> *n = new Student<5>;

    delete n;
    return 0;
}
francesco
  • 7,189
  • 7
  • 22
  • 49
0

The problem is in this part of the code:

int main()
{
   Student *n = (*Student) malloc(sizeof(n));
   return 0;
}

Understanding that '*' operator is an indirection operator in C++ would help in identifying the problem. https://msdn.microsoft.com/en-us/library/caaw7h5s.aspx

You are not trying to extract value of Student structure, instead you are trying to type cast void * returned from malloc. Correct way to do it is:

Student *n = new Student (correct C++ way of doing things - using new operator)

This will give you a pointer to dynamically created Student structure. If you intend to create a dynamic array of Student, you would do:

Student *n = new Student[5]

Depending on your usage, you might want to consider an easy and C++ STL way of doing as: std::vector<Student> or std::list<Student>.

Look up for STL (Standard Template Libraries) in C++ and you should be able to get more details.

Shrikanth N
  • 652
  • 3
  • 17
  • You cannot use ```malloc``` to allocate an object like ```Student``` which contains a complex type ```string```. See the discussion https://stackoverflow.com/questions/40434360/c-memory-error-when-using-malloc-realloc-free-on-stdstring You must use ```new``` instead. – francesco Nov 14 '18 at 08:58
  • Thanks for pointing it out, have modified my answer accordingly. Although `malloc` works its not the right way of doing it in C++. It would not call the constructor etc. It might still work in this case as it is a simple struct. But yes, what you said is a good practice to follow in C++ – Shrikanth N Nov 14 '18 at 11:32
  • Just to be clear: in this case ```malloc``` does *not* work because the struct here contains a ```string```. A code with ```malloc``` will compile, but it will not correctly call the constructor of ```string```, hence paving the way for undefined behavior of segmentation fault. – francesco Nov 14 '18 at 17:16
  • Undefined behaviour is truly insidious. I would fall on my knees and praise any deity who used their divine powers to make un-or improperly initialized pointers always segfault. Yes, segfault is the most likely outcome on a modern CPU, but the less likely cases tend to be subtle and cause damage to the program that is extremely hard to diagnose. The worst among these do no visible damage and appear to work until they fail catastrophically at the worst possible time. – user4581301 Nov 14 '18 at 18:42
-2

If you are in c++, don't use "malloc" to allocate memory if you can use "new":

Student* student = new Student();

In addition to allocating space, "new" will also call the struct's constructor which will allow for initialization. The syntax is also less error-prone.

You'll also notice my variable naming. In my example I chose "student" instead of "n" because it's more descriptive to the reader.

MaddawgX9
  • 131
  • 8