[C++] Problem "Segmentation Fault"

[byte_head]

Hi,

ich denke der Fehler in folgendem Programm hat mit den Pointern zu tun, was mich aber verwirrt ist, dass Subtraktion und Division funktionieren. Bei 1 und 3 dagegen -> Error

#ifdef HAVE_CONFIG_H
#include <config.h>
#endif

#include <iostream>
#include <cstdlib>

using namespace std;

void addition(int,int,int*);
void subtraktion(int,int,int*);
void multiplizieren(int,int,int*);
void dividieren(int,int,int*);

int main()
{
	cout << "Bitte wählen Sie die gewünschte Operation: \n" << endl;
	cout << "1 = Addieren \n2 = Subtrahieren \n3 = Multiplizieren \n4 = Dividieren\n" << endl;
	int eingabe;
	cin >> eingabe;
	if(eingabe < 5 && eingabe != 0)
	{
	
		if(eingabe==1)
		{
			int a,b;
			int * erg;
			cout << "Bitte geben Sie die 2 zu addierenden Zahlen ein: " << endl;
			cin >> a;
			cin >> b;
			*erg=0;
			addition(a,b,erg);							// ohne void vor dem Funktionscall!! ohne * !?
			cout << a << " + " << b << " = " << *erg << endl;
		}
  	 	
		if(eingabe==2)
		{
			int a,b;
			int * erg;
			cout << "Bitte geben Sie die 2 zu subtrahierenden Zahlen ein: " << endl;
			cin >> a;
			cin >> b;
			*erg=0;
			subtraktion(a,b,erg);
			cout << a << " - " << b << " = " << *erg << endl;
		}
	
		if(eingabe==3)
		{
			int a,b;
			int * erg;
			cout << "Bitte geben Sie die 2 zu multiplizierenden Zahlen ein: " << endl;
			cin >> a;
			cin >> b;
			multiplizieren(a,b,erg);
			cout << a << " x " << b << " = " << *erg << endl;
		}

		if(eingabe==4)
		{
			int a,b;
			int * erg;
			cout << "Bitte geben Sie die 2 zu dividierenden Zahlen ein: " << endl;
			cin >> a;
			cin >> b;
			dividieren(a,b,erg);
			cout << a << " : " << b << " = " << *erg << endl;
		}
		
		} 

	else {	cout << "Error!" << endl;  }		// Falls der User die Auswahl ignoriert !

	return 0;
}

void addition(int z1, int z2, int* erg)
{
	*erg=z1+z2;
}

void subtraktion(int z1, int z2, int* erg)
{
	*erg=z1-z2;
}

void multiplizieren(int z1, int z2, int* erg)
{
	*erg=z1*z2;
}

void dividieren(int z1, int z2, int* erg)
{
	*erg=z1/z2;
}

Danke schonmal.

 

[Götterwind]

Das Ergebnis einer Division wird kein "int" sein Aber, das ist ein anderes Problem. Warum das explizit nicht mit addition und multiplikation funzt... mmhh...

Ah, alles klar! Normalerweise muss man dem Pointer erstmal nen Speicher zuweisen. Ansonst geht es ganz schön in die Hose. Am Besten per cast machen. Zum Schluß natürlich wieder free(pointer) machen

		if(eingabe==1)
		{
			int a,b;
			int * erg;
			cout << "Bitte geben Sie die 2 zu addierenden Zahlen ein: " << endl;
			cin >> a;
			cin >> b;
			erg = (int *)calloc(1,sizeof(int));
			addition(a,b,erg);							// ohne void vor dem Funktionscall!! ohne * !?
			cout << a << " + " << b << " = " << *erg << endl;
			free(erg);
		}

calloc bewirkt im Gegensatz zu malloc, dass der Wert an der Speicherstelle *erg (mit der Adresse &erg) schon 0 ist. Das wäre zumindest c-style...

 

[TK-Shockwave]

Wieso deklarierst du a und b nicht im Kopf, so sparst du bei jeder if Schleife Code?
Das Design des Programms ist seltsam, nimm doch besser ein switch () case Anweisung/block
a und b sowie reg würd ich mit 0 initialisieren.

Und schon funktioniert das Programm bei mir ohne "segmentation fault"

 

[Götterwind]

@TK-Shockwave
Das hängt vom vernwendeten Compiler ab, ob er intelligent genug ist Bei Visual C++ als Express Edition kommt immer noch ein schöner Absturz (zumindest ohne Speicherallokierung)!

 

[TK-Shockwave]

Ich hab es mit g++ 4.1 und VC++2005 Express kompilieren können.
Hab das SP1 von Visual Studio 2005 Express Edition mit drauf.

ggf. liegt es am lokal initialisierten Pointer * erg zu tun ...
Aber Formal sind dort keine Fehler, zumindest seh ich keinen.

 

[Götterwind]

Wie gesagt, ich habe es mit Visual Studio Express (incl. SP1) ausprobiert. Ohne allozierten Speicher schmiert es ab (compilieren geht, schmiest aber Warnungen raus beim ersten Mal). Sollte ja auch klar sein, zumindest für mich, der nur in C arbeitet... Wenn man einen Pointer benutzt - immer Speicher allozieren und wieder freigeben. Ist halt etwas lästig zu schreiben aber sehr effizient. Das wäre die Sichere Lösung. Ob man bei C++ noch allozieren muss, k.A..
Es stimmt aber, dass es besser wäre die Variablen, wenn möglich, schon am Anfang zu definieren.

Für so eine kleine Aufgabe wäre das Arbeiten mit Rückgabewert besser anstatt mit Pointern. Auch ist immer noch das Problem mit der Division da (1/2 != int-wert)

 

[Simpson474]

Ich denke er wills eher so machen - ist ein bisschen effizienter

if(eingabe==1)
{
        int a,b;
	[B]int erg[/B];
	cout << "Bitte geben Sie die 2 zu addierenden Zahlen ein: " << endl;
	cin >> a;
	cin >> b;
	addition(a,b, [B]&[/B]erg);							
	cout << a << " + " << b << " = " << [B]erg[/B] << endl;
	//free(erg); natürlich gehört das free weg, ist schon klar
}

Zur Erklärung: mit "int *erg" hast du einen Zeiger definiert, der standardmäßig auf eine zufällige Speicheradresse zeigt. Wenn du jetzt die Zeile "*erg = 0;" ausführst, dann wird an die zufällige Speicheradresse 0 geschrieben. Unter DOS hätte das wunderbar funktioniert, nur vielleicht hättest du Teile des Betriebssystems überschrieben und das System wäre früher oder später abgestürzt. w2k erlaubt jedoch nur den Zugriff auf den Programmeigenen Speicherbereich und dein zufälliger Zeiger zeitgt zufällig nicht in diesen Speicherbereich.

Zu meiner Lösung: man deklariert einfach eine Variabel, die groß genug ist, dass Ergebnis aufzunehmen (in deinem Fall ein int). Dann holt man sich mit dem &-Zeichen die Adresse dieser Variabel und übergibt diese der Funktion. Die Funktion füllt den Speicher an den die Adresse zeigt mit dem Ergebnis und alles sollte funktionieren.

 

[3dom F!ghter]

int * erg = new int;

durch

int *erg;

ersetzen wirkt hier wahre Wunder

MFG

 

[Götterwind]

Hier wird dir free(erg) zum Verhängnis. Wird sich nicht kompilieren lassen Du hast ja mit "int erg;" dem rechener die Arbeit für die Allozierung übertragen - damit hat man keinen Pointer im eigentlichen Sinne - damit auch kein free() möglich Lass also das free(erg); weg und es geht...

 

[Simpson474]

Wieso benutzt ihr alle dynamische Speicherallokierung. Lernt ihm doch wenn dann was sinnvolles und nicht die langsamste Möglichkeit, wie man das Problem lösen kann.

 

[3dom F!ghter]

ich glaube er benutz nicht ohne Grund Pointer

 

[Simpson474]

Pointer haben rein gar nichts mit dynamischer Speicherallokierung zu tun.

 

[TK-Shockwave]

Wenn schon Pointerinitialiserung dann bitte:
int *erg=NULL;

new int wird aber nicht bei ANSI C++ benutzt...

Das Problem wird klar die Pointerinitialisierung sein, die so auf irgendeinen Speicherbereich liegt.

 

[byte_head]

Also ich wollte das auch erst mit &erg machen, allerdings habe ich das nicht hinbekommen. :/

void addition(int,int,?);

funktion(a,b,&erg);

void addition (int a, int b, &erg)
{
...
}

Habe jetzt einfach mal rumprobiert, aber irgendwie will das nicht, definiere immer mindestens eins der drei wichtigen Dingen falsch... :|

 

[3dom F!ghter]

Pointer haben rein gar nichts mit dynamischer Speicherallokierung zu tun.

Das ist mir durchaus bewusst,aber es ist halt eine einfache Lösung und die Geschwindigkeit eines einfachen Taschenrechners ist zu vernachlässigen (jaja, komm mir nur mit dem "schlechten Programmierstil" ), aber du hast Recht per Referenz ist das wohl am besten.

 

[Götterwind]

Entweder du machst es wie ich in post #2 geschrieben habe, oder du machst es wie Simpson474 vorgeschlagen hat:

void addition(int,int,int*);
...
int main() {
...
		if(eingabe==1)
		{
                        ...
			addition(a,b, &erg);							
                        ...
		}
  	 	
...
}//end main

...

void addition(int z1, int z2, int* erg)
{
	*erg=z1+z2;
}

Viel einfacher wäre es mit einem return(wert) zu arbeiten. Dann brauch man keine Pointer Dazu wären dann die Funktionen aber nicht als void zu deklarieren!

 

[ritsche]

In diesem Fall sehe ich keinen Sinn einen Pointer zu verwenden.
Machs entweder so wie Simpson474 vorschlägt, oder verwende eine Referenz:

void addition(int z1, int z2, int& erg)   [COLOR="Red"]<- mit "int& erg" definierst du erg als Referenz auf die übergebene Variable[/COLOR]
{
	erg=z1+z2;  [COLOR="Red"]<-- kein * zur Derefernzierung mehr nötig![/COLOR]
}

if(eingabe==1)
{
        int a,b;
	int erg;
	cout << "Bitte geben Sie die 2 zu addierenden Zahlen ein: " << endl;
	cin >> a;
	cin >> b;
	addition(a,b, erg);		<--	[COLOR="Red"]kein "&" Zeichen hier vor dem erg!	[/COLOR]			
	cout << a << " + " << b << " = " << erg << endl;
	//free(erg); natürlich gehört das free weg, ist schon klar
}

Oh, und klar, wie Götterwind schreibt, die einfachste Lösung ist natürlich mit einem return Wert zu arbeiten.
Das sähe dann so aus:

int addition(int z1, int z2)  
{
	return z1+z2;    // simple, hm? ;-)
}


if(eingabe==1)
{
        int a,b;
	int erg;
	cout << "Bitte geben Sie die 2 zu addierenden Zahlen ein: " << endl;
	cin >> a;
	cin >> b;
	erg = addition(a,b);				
	cout << a << " + " << b << " = " << erg << endl;
}

 

[Simpson474]

Um eins mal klarzustellen: Das von mir sind keine Referenzen, das sind Pointer. Und für alle die jetzt noch ein Beispiel mit Referenz suchen, hier bitte:

void addition(int z1, int z2, int [B]&erg[/B])
{
	erg=z1+z2;
}


if(eingabe==1)
{
        int a,b;
	int erg;
	cout << "Bitte geben Sie die 2 zu addierenden Zahlen ein: " << endl;
	cin >> a;
	cin >> b;
	addition(a,b, [B]erg[/B]);							
	cout << a << " + " << b << " = " << erg << endl;
}

EDIT: Ich war minimal zu langsam

 

[Götterwind]

Wo wir wieder bei dem alten Leid von C und C++, den Pointern, sind. Ach ja, ich liebe die Dinger einfach...

 

[3dom F!ghter]

Oh ,stimmt ich bin schon völlig durcheinander
naja das wäre ja aber jetzt geklärt.