trouble with STL list initialization inside nested structure
M A
Please see following code.
It gives me segmentation fault when I try to add element to the list
on the very last line. I am not sure why the "bm" list inside MyStruct
is not getting initialized? What can I do to initialize it?
Thanks.
-------------------------------------------
#include <stdio.h>
#include <list>
#include <vector>
#include <iostream>
#include <algorithm>
using namespace std;
struct row {
int rowid;
char *data;
bool operator<(const struct row &a) const {
return (this->rowid < a.rowid);
};
};
typedef struct {
list<struct row> bm;
vector<struct row> vbm;
} MyStruct;
typedef struct pattern {
int nodenum; // unique in the graph
MyStruct ms;
} TP;
int main(int args, char **argv)
{
TP *tp = (TP *) malloc (sizeof(TP));
struct row r1 = {1, (char *)"xyz"};
tp->ms.bm.push_back(r1);
}
-------------------------------------------
Joshua Maurice
> Hi,
>
> Please see following code.
> It gives me segmentation fault when I try to add element to the list
> on the very last line. I am not sure why the "bm" list inside MyStruct
> is not getting initialized? What can I do to initialize it?
>
> Thanks.
[snip code]
> TP *tp = (TP *) malloc (sizeof(TP));
this is not the same as
TP *tp = new TP;
The first does not call any constructors. The second calls TP::TP, the
constructor, which also calls the constructor of all sub-objects,
including the std::list member sub-object. Without calling the
constructor of std::list, attempting to do anything with it is
undefined behavior. std::list::list() constructor sets up invariants
and internal data members.
You seem to be coming from a C background. I'd suggest a picking up a
good book on C++.
M A
differences between "malloc" and "new"!
I just made a quick fix as follows:
------------------------------------
typedef struct pattern {
int nodenum; // unique in the graph
MyStruct ms;
} TP;
int main(int args, char **argv)
{
TP *tp = new TP;
struct row r1 = {1, (char *)"xyz"};
tp->ms.bm.push_back(r1);
list<struct row>::iterator itr = tp->ms.bm.begin();
cout << (*itr).rowid << " " << (*itr).data << endl;
}
-------------------------------------
And it is working now.
Thanks a lot for your help. :)
Francesco S. Carta
> Thanks for the explanation Joshua. I wasn't aware of intricate
> differences between "malloc" and "new"!
> I just made a quick fix as follows:
> ------------------------------------
>
> typedef struct pattern {
> int nodenum; // unique in the graph
> MyStruct ms;
>
> } TP;
>
> int main(int args, char **argv)
> {
>
> TP *tp = new TP;
>
> struct row r1 = {1, (char *)"xyz"};
>
> tp->ms.bm.push_back(r1);
>
> list<struct row>::iterator itr = tp->ms.bm.begin();
>
> cout << (*itr).rowid << " " << (*itr).data << endl;}
>
> -------------------------------------
> And it is working now.
> Thanks a lot for your help. :)
Consider replacing C-style strings with std::string - if you really
come from a C background you will have to put apart some of the tricks
needed by C. Most of the explicit memory management gets done behind
the curtains by the STL containers, take advantage of them, when you
write in C++.
Also, learn to replace all C-style casts with C++ explicit casts -
you'll learn also that most of the times you don't need casting at
all. For example, conversions get normally done with functional
notation:
-------
int i = 10;
double d = double(i) / 3;
-------
Once I'm here: don't top post. It messes up replies and some users
gets a bit upset for them.
You can learn a lot of things about C++ and about comp.lang.c++ by
reading the FAQ: http://www.parashift.com/c++-faq-lite/
Have good time,
Francesco
--
Francesco S. Carta, hobbyist
http://fscode.altervista.org
SG
>
> typedef struct {
> list<struct row> bm;
> vector<struct row> vbm;
>
> } MyStruct;
This is a mixture of C and C++. You could write just as well
struct MyStruct {
list<row> bm;
vector<row> vbm;
};
> typedef struct pattern {
> int nodenum; // unique in the graph
> MyStruct ms;
> } TP;
same here
> int main(int args, char **argv)
> {
> TP *tp = (TP *) malloc (sizeof(TP));
> struct row r1 = {1, (char *)"xyz"};
> tp->ms.bm.push_back(r1);
> }
same here. Replace malloc with new and char* with std::string. Also, I
don't see a reason for using the free store. This should work too:
int main(int args, char **argv)
{
TP tp;
row r1 = {1, "xyz"}; // note: no "struct" prefix
tp.ms.bm.push_back(r1);
}
By the way: In C++ string literals are const. So, you better use
"const char*" instead of "char*". The latter one still compiles for
backwards compatibility to C. Since you're now allowed to modify the
characters of a string literal (not even in C) you'll only gain from
using const qualifiers here.
One last note: Try to improve encapsulation.
Cheers,
SG
Yannick Tremblay
M A <medha...@gmail.com> wrote:
>Thanks for the explanation Joshua. I wasn't aware of intricate
>differences between "malloc" and "new"!
>I just made a quick fix as follows:
>------------------------------------
>
>typedef struct pattern {
> int nodenum; // unique in the graph
> MyStruct ms;
>} TP;
>
>int main(int args, char **argv)
>{
>
> TP *tp = new TP;
>
> struct row r1 = {1, (char *)"xyz"};
>
> tp->ms.bm.push_back(r1);
>
> list<struct row>::iterator itr = tp->ms.bm.begin();
>
> cout << (*itr).rowid << " " << (*itr).data << endl;
>}
>-------------------------------------
>And it is working now.
>Thanks a lot for your help. :)
Correction: you think it is working and it has not segfaulted on you
yet because in your test code you are using a static C string...
Let's look at it again:
> struct row {
> int rowid;
> char *data;
> bool operator<(const struct row &a) const {
> return (this->rowid < a.rowid);
> };
> };
OK, C++ containers use value semantic and copy data around. So what
will happen when you put this in a container is that the row will be
shallow copied element by element:
row r1 = { 1, (char *)"xyz" };
row r2 = r1;
now r1.data points to the "xyz" static string and r2.data point to the exact
same memory address. So if you modify the content of r1.data, you are
also modifying r2.
Things will become much worse if instead of using a static string
"xyz", you use some dynamic buffer inadvertently. Note that
dynamically allocating your char *data will not solve your problem,
only make it more complicated and create heap violation rather than
stack violation.
E.g.:
int main()
{
char *buffer = strdup("Oh dear!");
int i = 84;
TP tp;
row r1 = { i, buffer };
tp.ms.bm.push_back(r1);
// ...
free(buffer);
// ...
list<row>::iterator itr = tp->ms.bm.begin();
printf("%s\n", itr->data); // !!! Oh dear!
}
The simplest fix is:
struct row
{
int i;
std::string data;
bool operator<(const row &a) const {
return (this->rowid < a.rowid);
};
}
The alternative is to implement it all yourself with a constructor, a
copy constructor and an assignement operator that allocate a buffer
for data and copy the input as well as a destructor that release the
memory. But that's a waste of effort.
>typedef struct {
> list<struct row> bm;
> vector<struct row> vbm;
>} MyStruct;
C+, remove the unecessary typedef and struct:
struct MyStruct {
list<row> bm;
vector<row> vbm;
}
> typedef struct pattern {
> int nodenum; // unique in the graph
> MyStruct ms;
> } TP;
Again reomve unecessary typedef
>int main(int args, char **argv)
>{
> TP *tp = new TP;
Why dynamic allocation?
TP tp;
Works perfectly and should be the default. i.e. use new only when you must.
> struct row r1 = {1, (char *)"xyz"};
Unecessary struct
You are assigning r1.data to a static string. (unless you made the
change to use a std::string or created a contructor the strcpy the
input). This is likely to cause you problems as your program grow. I
don't think you are planning to only ever assing static C string to
rows, are you?
tp->ms.bm.push_back(r1);
}
Hope this helps. Unfortunately, moving from C to C++ means a lot of
unleanring to do.
Yannick