Segmentation fault, Problem mit new

[pumuckl]

Schau dir mal Referenzen an, das erleichtert die ganze Pointerfrickelei um einiges. Deine call-by-reference über Pointer zu machen ist C-Stil und in C++ nicht nötig.

[knivil]

Da wir hier in C++ sind, ist der Hinweis auf std::vector obligatorisch: Nutze std::vector (in Kombination mit Referenzen beim Funktionsparameter)!

[Simon2]

alx3400 schrieb:

Ich rufe doch in Zeile 30 die Funktion readgraph auf, dafür muss graph doch definiert sein, oder nicht? ...

Schon, aber das ist kein Grund, die Variable global zu machen. Außerdem: string-as-string-can...
Zusammen mit den Hinweisen auf vector und Referenzen:

#include <string>
#include <vector>

class GRAPH{
public:
    int degree;
    int *neighbours;
};

int readgraph(const char *filename, int &n, std::vector<GRAPH>& graph){
    ifstream in(filename);
    in >> n;
    cout << "Test: " << *n;
    graph.resize(n); // braucht's eigentlich nicht unbedingt
    graph[0].degree=1; // Achtung: Hier fehlen Größenüberprüfungen!!
    graph[1].degree=1;
    // was ist mit den Anderen graph-Objekten?
    return 0;
} 

int main(){
    std::string dateiname;
    cout << "Dateiname: ";
    cin >> dateiname;

    int n;                   // reicht vollkommen aus, die hier zu deklarieren
    std::vector<GRAPH> graph; // reicht vollkommen aus, die hier zu deklarieren
    readgraph(dateiname.c_str(), n, graph);
    return 0;
}

Gruß,

Simon2.

[alx3400]

Tachyon schrieb:

graph[0]->degree=1; //sollte (*graph)[0].degree=1; sein
graph[1]->degree=1; //sollte (*graph)[1].degree=1; sein

Das ist doch nur eine andere Schreibweise oder? Jedenfalls macht das Programm genau den gleichen Quatsch in beiden Schreibweisen.

Container sollte ich mir mal anschauen, und es steht auch schon auf der To-Do-Liste, aber im Moment ist es mir wichtig, das mit einem Array zu machen.

[Mizar]

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

class GRAPH{
public:
    int degree;
    int *neighbours;
};
// globale Variablen entfernt

bool readgraph(const char *filename, int &anzahl, GRAPH *&graph){ // Referenzen sind doch schicker ;)
    ifstream in(filename);
	if(!in)	// wurde die Datei überhaupt gefunden?
		return false;
    in >> anzahl;
    cout << "Test: " << anzahl << endl;
    graph = new GRAPH[anzahl];
	for(int i(0); i != anzahl; ++i) // alle Elemente initialisieren
		graph[i].degree = 1;
    return true;
}

int main(){
	std::string dateiname; // std::string
	int n;
	GRAPH *graph;
    cout << "Dateiname: ";
	getline(cin, dateiname); // ganze Zeile lesen, falls Dateiname leerzeichen enthält
	if(readgraph(dateiname.c_str(), n, graph)) { // dank bool'schen Rückgabewert überprüfen ob es geklappt hat
		cout << "Es wurden " << n << " Graphen erstellt." << endl;
		for(int i(0); i != n; ++i) {
			cout << graph[i].degree << " ";
		}
		delete [] graph; // nicht vergessen den Speicher auch wieder freizugeben
	} else {
		cerr << "Fehler beim lesen der Graphen." << endl;
	}

    return 0;
}

[Simon2]

alx3400 schrieb:

Tachyon schrieb:

graph[0]->degree=1; //sollte (*graph)[0].degree=1; sein
graph[1]->degree=1; //sollte (*graph)[1].degree=1; sein

Das ist doch nur eine andere Schreibweise oder? ...

Überleg' Dir mal, worauf Du jeweils den "[]" loslässt...

alx3400 schrieb:

Jedenfalls macht das Programm genau den gleichen Quatsch in beiden Schreibweisen....

Das liegt ziemlich sicher daran, dass Du "new[*n]" ausführst, ohne dass sichergestellt ist, dass n auf den richtigen Wert weist und/oder dass Du auf "graph[0]" und "graph[1]" zugreifst, ohne zu wissen, wie viele Elemente da wirklich liegen.

kleiner Tipp: Ich habe mein obiges Post nochmal überarbeitet ...

Gruß,

Simon2.

[alx3400]

Vielen Dank dafür. Aber kannst du mir auch sagen, warum das bei deinem Code geht und bei meinem nicht. Die entscheidende Stelle ist doch diese oder?

in >> anzahl; 
    cout << "Test: " << anzahl << endl; 
    graph = new GRAPH[anzahl]; 
    for(int i(0); i != anzahl; ++i) // alle Elemente initialisieren 
        graph[i].degree = 1;

Und die sah bei mir ja kaum anders aus. Mich wundert immer noch, wo der Fehler herkam.

[Tachyon]

alx3400 schrieb:

Das ist doch nur eine andere Schreibweise oder? Jedenfalls macht das Programm genau den gleichen Quatsch in beiden Schreibweisen.

Nein. graph[1]... liefert Dir graph** + sizeof(graph*) . Das Element gibt es nicht. Hinter graph steckt nur genau ein Element.

[It0101]

int *n

Das hier ist wohl auch nicht nötig. Er brauch die Länge des Arrays nicht als Pointer übergeben... Also selbst für ein "früheres" Ansi-C-Programm is das nich doll

[Mizar]

alx3400 schrieb:

Vielen Dank dafür. Aber kannst du mir auch sagen, warum das bei deinem Code geht und bei meinem nicht. Die entscheidende Stelle ist doch diese oder?

bool readgraph(const char *filename, int &anzahl, GRAPH *&graph)

Diese Funktion erwartet nun eine Referenz auf einen Zeiger, statt wie bisher:

int readgraph(const char *filename, int *n, GRAPH **graph)

Wie pumuckl auch schon sagte:

pumuckl schrieb:

Schau dir mal Referenzen an, das erleichtert die ganze Pointerfrickelei um einiges. Deine call-by-reference über Pointer zu machen ist C-Stil und in C++ nicht nötig.

wird es mit Referenzen leichter.

[AlexXXx]

Tachyon schrieb:

Nein. graph[1]... liefert Dir graph** + sizeof(graph*) .Das Element gibt es nicht. Hinter graph steckt nur genau ein Element.

Ich komme zum gleichen Schluß wie du =). Aber dein Weg ist anderst und sehr interessant. Weißt du wo die [] operatoren definiert sind in c++ also z.b. iostrem oder so. Ich würde sie mir gerne anschauen.
Gruß

[alx3400]

Erstmal vielen Dank für die vielen Antworten. Werd mir das jetzt erstmal in Ruhe anschauen.

[alx3400]

Habe nun einiges berücksichtigt, was mir hier empfohlen wurde und bin mit dem Einlesen des Graphen nun fertig. Wen es interessiert, hier der Code:

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

class GRAPH{ 
public: 
    int degree; 
    std::vector<int> neighbours; 
}; 
int i,n;

bool readgraph(const char *filename, int &n, std::vector<GRAPH>& graph){  
	int n1,n2;
    ifstream in(filename); 
	if(!in)
		return false;
    in >> n; 
	graph.resize(n);
    cout << "Anzahl Knoten: " << n << endl; 

    for(i=0; i < n; ++i) 		//degrees mit 0 initialisieren
        graph[i].degree = 0;		

	while(!in.eof()){				//degrees der Knoten ermitteln
		in >> n1 >> n2;	
		graph[n1].degree++;			
		graph[n2].degree++;
		cout << "\nn1: " << n1 << "\tn2: " << n2;
	}

	for (i = 0; i < n; i++){			//neighbour arrays initialisieren
		graph[i].neighbours.resize(graph[i].degree);
		graph[i].degree = 0;		
	}
	in.clear();						 //zurück auf Datei Anfang
	in.seekg (0, ios::beg);       
	in >> n;

	while (!in.eof())				//neighbour-vector füllen
      {
         in >> n1 >> n2;			
         graph[n1].neighbours[graph[n1].degree++] = n2;
         graph[n2].neighbours[graph[n2].degree++] = n1;
      }

    return true; 
} 

int print(std::vector<GRAPH>& graph){		//Testausgabe
	for (i = 0; i < n; i++){
		  int j;										
		  cout << "\nNeighbours of node " << i << ": ";
		  for (j = 0; j < graph[i].degree; j++){
			  cout << " " << graph[i].neighbours[j];
		  }
	}
	return 0;
}

int main(){ 
	std::vector<GRAPH> graph;
    std::string dateiname; 
    cout << "Dateiname: "; 
    getline(cin, dateiname); // ganze Zeile lesen, falls Dateiname leerzeichen enthält 
	if(readgraph(dateiname.c_str(), n, graph)){
		print(graph);
	}
		else{ 
			cerr << "Datei konnte nicht geoeffnet werden";
	}
    return 0; 
}

Bin immer offen für Verbesserungen, aber Fragen habe ich soweit keine mehr.
Danke nochmal

[unskilled]

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

class GRAPH
{
public:
	explicit GRAPH(int _degree = 0)
	:	degree(_degree)
	{}

	int degree; 

	std::vector<int> neighbours;
};

bool readgraph(const char *filename, std::vector<GRAPH> &graph)
{
    ifstream in(filename);
    if(!in) 
        return false;
	std::vector<GRAPH>::size_type len;
    in >> len;
    graph.resize(len);
    cout << "Anzahl Knoten: " << n << endl; 

	for(int n1, n2; in >> n1 >> n2; )
	{                //degrees der Knoten ermitteln 
        graph.at(n1).degree++;
        graph.at(n2).degree++;
    } 

    for (i = 0; i < n; i++)
	{            //neighbour arrays initialisieren 
        graph[i].neighbours.resize(graph[i].degree); 
        graph[i].degree = 0;        
    } 
    in.clear();                         //zurück auf Datei Anfang 
    in.seekg (0, ios::beg);
    in >> n;

	for(int n1, n2; in >> n1 >> n2; )
	{			//neighbour-vector füllen
		graph[n1].neighbours[graph[n1].degree++] = n2;
		graph[n2].neighbours[graph[n2].degree++] = n1;
	}

    return true; 
} 

void print(const std::vector<GRAPH>& graph)
{        //Testausgabe 
    for (int i = 0; i < n; i++)
	{
          cout << "\nNeighbours of node " << i << ": ";
          for (int j = 0; j < graph[i].degree; j++)
		  {
              cout << " " << graph[i].neighbours[j]; 
          } 
    } 
} 

int main()
{
    std::string dateiname; 
    cout << "Dateiname: "; 
    getline(cin, dateiname); // ganze Zeile lesen, falls Dateiname leerzeichen enthält 
    std::vector<GRAPH> graph; 
    if(readgraph(dateiname.c_str(), n, graph))
        print(graph); 
	else
		cerr << "Datei konnte nicht geoeffnet werden"; 
}

ab und an solltest du int noch durch den size_type ersetzen...
allerdings hab ich jz nur blind iwas geändert, weil ich nicht gelesen habe, was du eigtl machen möchtest - und es mir zu kompliziert erschien, den code so lang zu untersuchen, bis ich das verstanden hab^^
werd wohl auch paar fehler reingemacht haben^^

bb