Segmentation fault on instationation of more than 1 object

Question

I have a class called "Vertex.hpp" which is as follows:

 #include <iostream>
 #include "Edge.hpp"
 #include <vector>
   using namespace std;

/** A class, instances of which are nodes in an HCTree.
 */
class Vertex {
public:
Vertex(char * str){
 *name=*str;
 }

vector<Vertex*> adjecency_list;
vector<Edge*> edge_weights;
char *name;

 };

#endif

When I instantiate an object of type Vector as follows:

 Vertex *first_read;
 Vertex *second_read;

  in.getline(input,256);

  str=strtok(input," ");
  first_read->name=str;


  str=strtok(NULL, " ");
  second_read->name=str;

A segmentation fault occurs when more than 1 object of type Vector is instantiated. Why would this occur if more than 1 object is instantiated, and how can i allow multiple objects to be instantiated?

Please use `std::string`. That's not the way to use C strings (or pointers) at all. A [good book](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) would really help. — chris, Dec 04 '12 at 22:55
`first_read` and `second_read` are pointers to `Vertex` and are uninitialized (in the code you quote). Hence the crash. — Nik Bougalis, Dec 04 '12 at 22:56
Lots wrong, remove the "using namespace std" from the header, quit dereferencing an uninitialized pointer (name - in the constructor), uninitialized first_read and second_read. Start with some basic pointer usage and then revisit this. — Ryan Guthrie, Dec 04 '12 at 22:59
@RyanGuthrie: No, no, no, please don't start with *any* pointer usage! — Kerrek SB, Dec 04 '12 at 22:59
@Kerrek SB when it comes to learning C++ (which includes C), and this looks like homework, it is important to know how pointers work and not just use high level abstractions to hide that information. — Ryan Guthrie, Dec 04 '12 at 23:13
@RyanGuthrie: I'm not sure about that. Sometimes I think that people would write better code if they *didn't* know what pointers were and just didn't use them. Pointers should be an *advanced* topic if you want to take a low level of control because you know what you're doing... — Kerrek SB, Dec 04 '12 at 23:24

score 2 · Answer 1 · edited Dec 07 '12 at 01:56

2

*name=*str;

You cannot dereference a pointer until you first make it point to something.

You probably meant something like:

Vertex(char * str) {
    name=strdup(str);
}

But you should really be using std::string.

edited Dec 07 '12 at 01:56

answered Dec 04 '12 at 23:02

David Schwartz

179,497
17
214
278

score 0 · Answer 2 · answered Dec 04 '12 at 23:19

I think the way how you copy strings is wrong.

*name=*str;

Both name ans str are of type char*. You are dereferencing those pointers. This means you look at the position at the memory where they point and interpret it as char.

When you call it first time something is at location pointed by str and first character of it is copied to random address (as you never initialized name).

Second time you are not so lucky. strtok called on NULL return NULL strtok at cplusplus

Now you tried to work with memorry pointed by null pointer and it is bad.

You need to allocate memory for name and use proper copy function.

name = new char[SomeMaxLenght];
strcpy(name, str);

score 0 · Answer 3 · edited May 23 '17 at 11:48

That's a very C way of doing things, which is incredibly non recommended in modern C++. Remember, C++ should be treated as a different language, not a strict superset of C.

First things first, you should really get a good book by looking at that list, as you seem to be missing many basics.

As for your problem, the main issue is that name is uninitialized, so you run into what is called undefined behavior (i.e. anything can happen; in your case it crashes on the second instantiation). I could go in depth on how to fix it by dynamically allocating memory, but why bother? Just use a std::string:

class Vertex {
    std::string name; // string instead of char *
public:
    Vertex(const std::string &str) { // pass by const reference
        name = str; // I should really use an initializer list there, but oh well
    }

    // the rest of the class is the same
};

See how that's simpler? Now you don't have to mess around with pointers, which are painful to use. So, in short: use the standard library. And get a good book. Really.

@sehe Yeah, that was a pretty huge buildup just for that punchline. Although _at least_ I'm not suggesting pointers like everyone else around here. — Etienne de Martel, Dec 04 '12 at 23:25

Segmentation fault on instationation of more than 1 object

3 Answers3