realloc - fehlerhafter Wert im Array

snowy

Hallo,

Ich schreibe gerade ein kleines Beispielprogramm um den Umgang mit malloc,calloc und realloc zu lernen. Das ist ein Programm wo ein abstrakter Datentyp, ein Stack, implementiert werden soll.

Wenn der Stack (int - Array) zu klein ist für den push Befehl dann soll er mit realloc um 50 Einheiten vergrößert werden. Die Ausgangsgröße im bsp ist 5. Dann werden einfach die Zahlen 0 - 9 gepusht. Beim abholen der Werte müsste dann aber herauskommen: 9 8 7 6 5 4 3 2 1 0. Es kommt aber 9 8 7 6 5 -842150451 3 2 1 0.

Genau an der Stelle beim realloc geht die "4" verloren.

Hier der Code:

int stck_push(struct stack *stck, int val)
{
	int *new_space=NULL;

	if(stck->top < stck->size)
	{
		//Wert speichern
		stck->arrStack[stck->top+1] = val;
		stck->top++;
	}
	else
	{
		//Stack muss erweitert werden um 50 Elemente, es muss ein temporärer Zeiger verwendet werden (?)
		new_space = (int *)realloc(new_space,stck->size+50);
		if(new_space == NULL)
			return 0;

		//Inhalt kopieren von stck zu new_space
		for(int i=0; i < stck->top; i++)
			new_space[i] = stck->arrStack[i];

		//stck bekommt als zuweisung new_space mit gesichertem Inhalt
		stck->arrStack = new_space;

		//Größe anpassen
		stck->size+=50;

		//neuen Wert speichern
		stck->arrStack[stck->top+1] = val;
		stck->top++;
	}

	return 1;
}

Statt den temporären Zwischenzeiger zu verwenden (new_space) habe ich auch schon versucht, gleich direkt "stck" zu verwenden.Es kam das gleiche Ergebnis heraus.

Wo liegt mein Denkfehler?

Danke!

Hab den Code nur kurz überflogen:

//Stack muss erweitert werden um 50 Elemente, es muss ein temporärer Zeiger verwendet werden (?)
        new_space = (int *)realloc(new_space,stck->size+50);

Hier werden nicht 50 Elemente mehr geholt, sondern 50 Bytes! Korrekt wäre also (stck->size+50) * sizeof *new_space
Ich gehe mal davon aus, dass das beim Initialen reservieren auch schon falsch gemacht wurde.

Warum rufst du überhaupt realloc() mit einem NULL-Zeiger auf? Das manuelle kopieren der Daten weiter unten könntest du dir komplett sparen wenn du realloc() gleich wie gedacht verwendest. Hätte im übrigen auch den Vorteil, dass du dann nicht ständig Speicherleichen produzierst (hint: stck->arrStack = new_space; <- da verlierst du worauf arrStack vorher gezeigt hat).

supertux

ralloc benutzt du falsch.

else
    {

        new_space = realloc(stck->arrStack, stck->size+50*sizeof *stck->arrStack);
        if(new_space == NULL)
            return 0;

        /* das kopieren kannst du dir schenken, das garantiert realloc bereits */
        stck->arrStack = new_space;

        //Größe anpassen
        stck->size+=50;

        //neuen Wert speichern
        stck->arrStack[stck->top+1] = val;
        stck->top++;
}

Vicious Falcon

Außerdem

stck->arrStack[stck->top+1] = val;

sieht mir irgendwie verdächtig aus. Ich habe es nicht getestet, aber müsste es nicht

stck->arrStack[stck->top] = val;

heißen?

snowy

Vielen Dank erstmal für die zügigen Antworten.

Durch die Änderung:

new_space = (int *)realloc(stck->arrStack , stck->size+50 * sizeof *stck->arrStack);

Kommt das gleiche Ergebnis heraus.

Durch entfernen der +1 bei :

stck->arrStack[stck->top] = val;

Die +1 müsste doch richtig sein oder? weil bei top der letzte Wert drin steht.

kommt:
9 8 7 6 -842... 5 3 2 1 0

Muss ich überhaupt den temporären Zeigen new_space verwenden..?

Nick Unbekannt

Ich würde dir raten die Anweisungen für das hinzufügen des Werten aus der Abfrage raus zu nehmen. Du machst dort zweimal das gleiche, es könnte also sein, dass du dort nicht das +1 geändert hast.

snowy

int stck_push(struct stack *stck, int val)
{
	if(!(stck->top < stck->size))
	{
		//Stack muss erweitert werden um 50 Elemente
		stck->arrStack = (int *)realloc(stck->arrStack , stck->size+50 * sizeof(int));
		if(stck->arrStack == NULL)
			return 0;

		//Größe anpassen
		stck->size+=50;
	}

	//neuen Wert speichern
	stck->arrStack[stck->top] = val;
	stck->top++;

	return 1;
}

int stck_pop(struct stack *stck, int *val)
{
	if(!stck)
		return 0;

	if(stck->top > 0)
	{
		stck->top--;

		*val = (int)stck->arrStack[stck->top];

	}
	else
		return 0;

	return 1;
}

Ich habe den push-befehl eingekürzt. Das Problem lag daran top mit 0 initialisiert wird.Dann aber gleich beim ersten Durchgang 1 gesetzt wird. Damit gab es eine Verschiebung. Zusätzlich war im pop-Befehl top-- an einer falschen Stelle (nach *val=(int)stck...).

Jetzt geht es.

Vielen Dank!

Nick Unbekannt

Es ist übrigens üblich im Erfolgsfall eine 0 zurück zugeben und sonst den konkreten Fehler. Das macht die Fehlersuche einfacher.

#define NO_STACK         1
#define REALLOC_FAILED   2
...

Vicious Falcon

Du treibst dennoch ein gefährliches Spiel. realloc kann auch 0 zurückliefern, der Speicherbereich wird dann nicht freigegeben. Da du die alte Adresse überschreibst, kannst du mit ihm aber nichts mehr anfangen -> Inhalt verloren & Speicherleck entstanden.

snowy

@Falcon:

Ich teste doch auf NULL. Damit bekomme ich doch mit wenn es nicht geklappt oder?

manual:
"If there is not enough available memory to expand the block to the given size, the original block is left unchanged, and NULL is returned.
If size is zero, then the block pointed to by memblock is freed; the return value is NULL, and memblock is left pointing at a freed block."

@Nick:

Danke für den Tipp. Die defines habe ich in meinem Headerfile deklariert. Wie mach ich das wenn ich viele Funktionen habe mit einem zugehörigen Headerfile. Dann müsste ich ja die defines durchnummeriern..
Müsste ich dann pro Funktion ein enum anlegen mit den Funktionszugehörigen Fehlercodes?Oder wie macht man das richtig?

Nick Unbekannt

Richtig oder falsch kommt wohl immer auf das konkrete Problem an. Da die Funktionen aber alle auf das Stack-Objekt operieren, würde ich eine Liste mit möglichen Fehlern auf eben dieses Objekt anlegen. Eigene Fehlercodes pro Funktion halte ich sogar für eher ungünstig, so muss der Programmierer immer schauen, was bedeutet denn dieser Fehler an der Stelle. Auch geht dadurch die Bedeutung des Fehlers, wenn du ihn nach oben hin durch reichst verloren. Außer du bastelst eine Kennung mit rein, anhand derer man erkennen kann, wer den Fehler verursacht hat.

Pipboy

snowy schrieb:

Ich teste doch auf NULL. Damit bekomme ich doch mit wenn es nicht geklappt oder?

Das Problem ist, dass du stck->arrStack vor dem Vergleich schon NULL gesetzt hast.
Damit ist die Adresse des Speicherblocks weg und du kannst, wie schon gesagt, kein free() mehr aufrufen.
Was du brauchst ist ein:

stack *temp = realloc...
if(temp == NULL)
	//fehler
else
	stck->arrStack = temp;