C++ dynamic array of struc doubt

matiasar · 11-28-2008, 06:18 AM

I'm studing and making my first real life program in C++. I've got a doubt related with including a string type into a struct when I'm managing memory allocation dynamically.
Bellow I copy the test program I did which works ok. But if I try to use string type instead of "const char *" into the struct I receive a "segment violation" error when I run the program. No error messages when compilling it.

Does anybody knows if I'm doing something wrong?

#include <iostream>
using namespace std;

struct Datos {
const char * fecha;
const char * evento;
};

int main (void)
{
int max = 4;
int n = 0;

Datos * p_Datos = new Datos [4];

while (n < 9)
{
if ( n >= max)
{
max = max + 15;
Datos * p_temp = new Datos [max];

for (int i=0; i <= n; i++)
{
p_temp[i].fecha = p_Datos[i].fecha;
p_temp[i].evento = p_Datos[i].evento;
}
p_Datos = p_temp;
delete [] p_temp;
}

//Asignaciones
p_Datos[n].fecha = "25/11/2008";
p_Datos[n].evento = "Enviar e-mail!!!";
n++;
}

/* Muestra datos */

cout << "\n" << "Fecha\t\t" << "Evento";
for (int i=0; i < n; i++)
{
cout << "\n" << p_Datos[i].fecha << "\t\t" << p_Datos[i].evento;
}
cout << "\n\n";
return 0;
}

rizwanrafique · 11-28-2008, 07:52 AM

I think there are two problems in the program.

You allocate 4 locations to p_Datos using:

Code:

Datos * p_Datos = new Datos [4]

And then try to look to more than 4 using:
[CODE]
for (int i=0; i <= n; i++)
{
...
{/CODE]

Secondly:

Code:

delete [] p_temp;

Will clear up the memory allocated to p_temp even if it has been assigned to another pointer.

Fixing these two things runs the program properly.

My guest is that in const chat * version of the program the point nature of member variables stops memory from being cleared. But that's just a guess. Someone else can give a more definite answer :-)

matiasar · 11-28-2008, 08:36 AM

Thanks for your answer.
I tried the following code, but still I get "segment violation" when using string type into the struct. The same program using const char * works ok...

Code:

#include <iostream>
#include <string>
using namespace std;


struct Datos {
        string fecha;
        string evento;
};



int main (void)
{
        int max = 4;
        int n = 0;

        Datos * p_Datos = new Datos [4];

        while (n < 9)
        {
                if ( n >= max)
                {
                        max = max + 15;
                        Datos * p_temp = new Datos [max];

                        for (int i=0; i < n; i++)
                        {
                                p_temp[i].fecha = p_Datos[i].fecha;
                                p_temp[i].evento = p_Datos[i].evento;
                        }
                        p_Datos = p_temp;
                        delete [] p_temp;
                }

             //Asignaciones
             p_Datos[n].fecha = "25/11/2008";
             p_Datos[n].evento = "Enviar e-mail!!!";
             n++;
        }

        /*  show data */

        cout << "\n" << "Fecha\t\t" << "Evento";
        for (int i=0; i < n; i++)
        {
                cout << "\n" << p_Datos[i].fecha << "\t\t" << p_Datos[i].evento;
        }
        cout << "\n\n";
        return 0;
}

johnsfine · 11-28-2008, 09:28 AM

Quote:

Originally Posted by matiasar

Thanks for your answer.
I tried the following code,

That doesn't look like you did anything to fix the second problem rizwanrafique described.

Quote:

I get "segment violation" when using string type into the struct. The same program using const char * works ok.

The program is not correct even using const char*.
I understand why those serious bugs typically won't result in an immediate crash using const char*, but will using string. However, you are closer than you think to a crash even with the const char * version.

1) What you seem to be doing is inventing your own std::vector. I have done that in many places, because I know how and because I know which situations benefit enough from coding better than std::vector to be worth the trouble. You would be better off just using a std::vector<Datos> instead of reinventing that yourself.

2) If you must write it yourself, you need to be more systematic about the process of switching to the new copy. I suggest:

Code:

                if ( n >= max)
                {
                        int old_max = max;
                        Datos* old_Datos = p_Datos;
                        max = max + 15;
                        p_Datos = new Datos [max];

                        for (int i=0; i < old_max; i++)
                        {
                                p_Datos[i].fecha = old_Datos[i].fecha;
                                p_Datos[i].evento = old_Datos[i].evento;
                        }
                        delete [] old_Datos;
                }

Unlike your version, this one deletes the correct buffer (the old one, not the new one) and this one makes the limit of reading the old buffer explicit rather than relying on the purely single increment growth in n.

But if you want a minimal change, you could just delete the right buffer. Instead of:

Code:

                        p_Datos = p_temp;
                        delete [] p_temp;

use

Code:

                        delete [] p_Datos;
                        p_Datos = p_temp;

matiasar · 11-28-2008, 12:54 PM

I understood. Thanks a lot for your help and patience. I finally got it working using your approach Johnsfine.
I'll see if I keep on practicing this issue.

Regards,
Matías