dynamic memory allocation for array of structures

Question

I want to dynamically allocate for an array of structure 'student'`

struct student
     { 
     char* name;
};
int main(int argc,char* argv[])
{
   if(argc==2)
   {
     student* sptr=new student[4]; 
     for(int i=0;i<atoi(argv[1]);i++)
     {
         cin>>*(*(sptr+i)).name;
     }
     for(int i=0;i<atoi(argv[1]);i++)
     {
         cout<<*(*(sptr+i)).name;
     }
   }
}

the code is compiling without error,but at runtime, after entering first name is says "segmentation fault(core dumped)" and halts.where i am going wrong?

Answer 1

You are getting a segmentation because your program has undefined behaviour. There is an error in your code which your compiler is not required to diagnose; it instead generates a binary which can do anything, including random crashes.

Depending on user input, your problems are starting with the use of atoi in this line:

for(int i=0;i<atoi(argv[1]);i++)

First of all, your program can obviously only work if the user enters "4", which makes me wonder why you need user input anyway. Chances are that you wanted to write new student[atoi(argv[1]] in the line above.

Nevertheless, atoi is a dangerous function and should almost never be used. As documentation says:

If the converted value falls out of range of corresponding return type, the return value is undefined.

Which means that you will never know if the user entered a harmless number like "1" or something like "10000000000000000000000000", which is probably larger than the maximum int value on your machine.

What's possibly worse:

If no conversion can be performed, ​0​ is returned.

Which means you will never know if the user entered "abc" or "0".

std::stoi is a safe, modern alternative which allows you to perform error checking.

Still, let's assume that the user simply enters a "4".

We then enter into the loop body and encounter the following line:

cin>>*(*(sptr+i)).name;

Readability issues aside, what's happening here is:

• i is still 0, so you get (sptr+0), which is equal to (sptr).
• sptr is dereferenced to obtain a reference to a student object.
• You try to dereference the name pointer of the student object.

The latter step finally results in undefined behaviour, because the name pointer was not initialised. You must not do this. All the rest of your program has been rendered invalid at this point.

Getting this right with bare pointers is extremely difficult. You could add user input asking for the size of each name, so that you can allocate enough memory before reading the name. Or you could employ an even more complex approach using low-level member functions of std::istream.

Fortunately, you don't have to do any of this. This is C++: use std::string, std::vector and std::getline. Use C++ and not C if you want to write C++ and not C, and all your problems will disappear:

#include <iostream>
#include <string>
#include <vector>
#include <exception>

struct student
{ 
    std::string name;
};

int main(int argc, char* argv[])
{
   try
   {
       if (argc == 2)
       {
           std::vector<student> students(std::stoi(argv[1]));
           for (auto&& student : students)
           {
               std::getline(std::cin, student.name);
           }

           for (auto&& student : students)
           {
               std::cout << student.name << "\n";
           }
       }
    }
    catch (std::exception const& exc)
    {
        std::cerr << exc.what() << "\n";
    }
}

This program has defined behaviour; it will not crash with wrong input, and it will allow names with spaces in them. It is also easier to read, does not use any pointers on the outside, and it handles memory automatically, smarter and faster than manual code.

Answer 2

segment fault is you are accessing a memory location without initialize it. You need not to define a struct. Since it is CPP, so why not try CPP std::string data type.

And the name attribute in student struct is never allocated memory in your code.

try:

string * st = new string[atoi(argv[1])];
for (int i = 0; i < atoi(argv[1]); i ++)
{
    cin >> st[i];
    cout << st[i] << endl;
}

or

std::vector<std::string> name;

Answer 3

If you need to store names from input, you just need to use a data structure like vector or list of strings to avoid errors like memory managment:

std::vector<std::string> names;
// std::list<std::string> names;

Answer 4

You are getting a seg fault because you are trying to access uninitialized memory (the pointer name is never initialized) as pointed out by πάντα ῥεῖ and Joachim Pileborg.

Using std::string is the way to go; however, if this is an exercise or for whatever reason you should be using a char *, then it needs to be initialized for each of the array item separately in a loop.

student* sptr=new student[4]; 
for(int i=0;i<atoi(argv[1]);i++)
{
    sptr[i].name = new char[10]; // now name is properly initialized and can hold a string of length (10 - 1)
    cin >> sptr[i].name;
}

And when printing out the string, you should be doing

cout << sptr[i].name;

This is the same as (*(sptr+i)).name. This gives the char * that holds the string, if you add another * in front of the expression than it dereferences the pointer and gives you the first character of the string. So if you were to print only the first character, then your expression is just fine. However, to print out the whole string you should not dereference the pointer by adding another * to the beginning.

Answer 5

1. You don't need a struct to hold only one string
2. You don't need to call atoi in each iteration
3. You need to allocate memory in order to receive user input

A working code could be as following:

int main(int argc,char* argv[])
{
    // Exit if wrong number of arguments
    if(argc != 2)
    {
        cout << "Incorrect number of argements";
        return 1;
    }

    // Convert argument from ASCII to int once
    int numberOfValues = atoi(argv[1]);
    // Create array of strings (pointer of pointers to characters)
    char** ppValues = new char*[numberOfValues];

    // Loop to collect strings
    for (int i = 0; i < numberOfValues; i++)
    {
        // Local variable to collect one string
        char buffer[1024];
        cin >> buffer;
        // Allocate memory in one of the array entries to hold the current entry
        ppValues[i] = new char[strlen(buffer) + 1];
        // Copy value from temp string to final string location
        strcpy(ppValues[i], buffer);
    }

    for (int i = 0; i < numberOfValues; i++)
    {
        cout << ppValues[i];
    }

    return 0;
}