Strings aus einer verketteten Liste löschen

[TDM]

hi,

ich habe mir ein Programm für Arbeitsaufträge geschrieben...

sprich das soll den ablauf einer arbeit anzeigen...

stdflifo.h

#include <stdio.h>


#if !defined(CASE) || !defined(DEFAULT)

#define CASE break;case

#define DEFAULT break;default

#endif


#if !defined(CLS)

#include <stdlib.h>

#define CLS system("CLS")

#endif


#define MAX_ATTEMPT 3												/* Anzahl der Fehlversuche */

#define FEHLEINGABE CLS; printf("ungueltige Eingabe\n\n");Quit--


#define VL_MODE_DEFAULT	1


#ifndef VL_MODE

#define VL_MODE VL_MODE_DEFAULT

#endif


#ifndef MAX_STRLEN

#define MAX_STRLEN 4096

#endif


//Struct Deklaration

struct sNode;


//Typedef

#ifndef Element_ID

typedef int		Element_ID;

#endif

#ifndef Element

typedef char	*Element;

#endif

typedef sNode	*Link;											/* Link = Verweise */


//Struct Definition

struct sNode 

{

	Element_ID ID;

	Element Elem;

	Link    PPrev;

	Link    PNext;

};


#define NEW_ADDRESS (Link) malloc(sizeof(sNode))


#if !VL_MODE

//Kopf und Schwanz (erste und letzte)

Link head = NEW_ADDRESS;

Link tail;

#else 

Link tail = NEW_ADDRESS;

#endif


//Funktion

void print_menu(unsigned int Orders)

{

		printf("********************************************************************************"

		       "*                                                                              *"

		       "*                                   1 = Neuen Auftrag erzeugen                 *");

		if (Orders)

		printf("*                                   2 = Aktuellen Auftrag anzeigen             *"

			   "*                                   3 = Bestimmten Auftrag anzeigen            *"

		       "*                                   4 = Aktuellen Auftrag loeschen             *"

			   "*                                   5 = Bestimmten Auftrag loeschen            *");

		printf("*                                   6 = Programm beenden                       *"

		       "*                                                                              *"

			   "*                                   Noch %d Autraege                            *"

		       "********************************************************************************",Orders);

}


/*//////////////////////////////////////////////////////////////////

// LIFO Funktionen

//////////////////////////////////////////////////////////////////*/


#if !VL_MODE

void push(Element data)

{

	Link tmp;

	tmp = NEW_ADDRESS;

	tmp -> Elem = data;												/* Daten eintragen */

	if (!head -> Elem)

	{

		head = tmp;

		tmp -> ID = 1;

		tmp -> PPrev = NULL;										/* Anfang auf NULL setzen */

	}

	else

	{

		tmp  -> PPrev = tail;										/* Verlinkung zum Vorherigen */

		tmp  -> ID    = (tmp -> PPrev -> ID + 1);

		tail -> PNext = tmp;										/* Verlinkung zum Nächsten */

	}

	tmp -> PNext = NULL;											/* Nächste vom Nächsten gibts nicht */

	tail = tmp;														/* Neues Ende */

}


inline Element show(void)

{

	return head -> Elem;

}


Element show(Element_ID ID)

{

	Link tmp = head;

	while (tmp -> ID != ID)											/*Solange ID nicht gefunden wurde*/

	{

		tmp = tmp -> PNext;

	}

	return tmp -> Elem;


}


inline void set_default(void)

{

	head -> Elem = NULL;

}


void pop(void)

{

	if (head -> PNext)												/* Wenn es ein nächstes gibt */

	{

		Link tmp;

		tmp = head -> PNext;

		tmp -> PPrev = NULL;

		free(head);

		head = tmp;

	}

	else if (head == tail && head -> Elem)							/* erste gleich letzte (nur noch 1 Element) */

	{

		head -> Elem = tail -> Elem = NULL;

		free(head);													/* Addresse freigeben */

	}

}


void pop(Element_ID ID)

{

	Link tmp = head;

	Link _tmp;

	while (tmp -> ID != ID)

	{

		tmp = tmp -> PNext;

	}

	_tmp = tmp;

	while (_tmp -> PNext)

	{

		_tmp = _tmp -> PNext;

		(_tmp -> ID)--;

	}

	if (tmp == head)

	{

		head = tmp -> PNext;

	}

	else

	{

		tmp -> PPrev -> PPrev = tmp -> PPrev;

	}

	tmp -> PPrev ->	PNext = tmp -> PNext;

	free(tmp);

}


/*//////////////////////////////////////////////////////////////////

// FIFO Funktionen

//////////////////////////////////////////////////////////////////*/


#else

void push(Element data)

{

	Link tmp;

	tmp = (Link) malloc(sizeof(sNode));

	tmp  -> Elem  = data;

	tail -> PNext = tmp;

	tmp  -> PNext = NULL;

	if (!tail -> PPrev)

	{

		tmp -> ID  = 1;

	}

	else

	{

		tmp  -> ID = (tail -> ID + 1);

	}

	tmp  -> PPrev  = tail;

	tail = tmp;

}


inline Element show(void)

{

	return tail -> Elem;

}


Element show(Element_ID ID)

{

	Link tmp = tail;

	while (tmp -> ID != ID)

	{

		tmp = tmp -> PPrev;

	}

	return tmp -> Elem;

}


inline void set_default(void)

{

	tail -> PPrev = NULL;

	tail -> Elem  = NULL;

	tail -> ID    = 1;

}


void pop(void)

{

	if(tail -> PPrev)												/*nicht das Letzte */

	{

		Link tmp;

		tmp = (Link) malloc(sizeof(sNode));

		tmp = tail -> PPrev;

		tmp -> PNext = NULL;

		free(tail);

		tail = tmp;

	}

	else

	{

		free(tail);													/* letzte löschen */

	}


}


void pop(Element_ID ID)

{

	Link tmp = tail;

	Link _tmp;

	while (tmp -> ID != ID)

	{

		tmp = tmp -> PPrev;

	}

	_tmp = tmp;

	while (_tmp -> PNext)

	{

		_tmp = _tmp -> PNext;

		(_tmp -> ID)--;

	}

	if (tmp -> PPrev && tmp -> PNext)

	{

		tmp -> PPrev -> PNext = tmp -> PNext;

		tmp -> PNext -> PPrev = tmp -> PPrev;

	}

	else if (!tmp -> PNext)

	{

		tmp -> PPrev -> PNext = NULL;

		tail = tmp   -> PPrev;

	}

	else

	{

		tmp -> PNext -> PPrev = NULL;

	}

	free(tmp);

}

#endif

FLIFO.c

//Einsprungspunkt für FIFO.c

#define VL_MODE 0

#undef __C++

#include "stdflifo.h"

#include <iostream>


using namespace std;


int main()

{

	short int Quit = MAX_ATTEMPT ; 

	int Orders = 0, Wahl;

	Element_ID ID;

	char data[MAX_STRLEN];

	set_default();

	while (Quit)

	{

		print_menu(Orders);

		scanf("%d", &Wahl);

		switch (Wahl)

		{

		case 1:

		     CLS;

			 printf("Zahl eingeben:\t");

			 scanf("%s", data);

			 push(data);

			 Orders++;

			 if (Quit < MAX_ATTEMPT) Quit = MAX_ATTEMPT;

		CASE 2:

			 if (Orders)

			 {

				CLS;

				printf("Aktueller Auftrag:\t%s\n\n",show());

				if (Quit < MAX_ATTEMPT) Quit = MAX_ATTEMPT;

			 }

			 else

			 {

				 FEHLEINGABE;

			 }

		CASE 3:

			 if (Orders)

			 {

				printf("AuftragsID bitte eingeben:\t");

				scanf("%i", &ID);

				CLS;

				if (ID > Orders)

				{

					printf("Dieser Auftrag existiert nicht.\n\n");

				}

				else

				{

					printf("%d. Auftrag:\t%s\n\n",ID,show(ID));

				}

				if (Quit < MAX_ATTEMPT) Quit = MAX_ATTEMPT;

			 }

			 else

			 {

				 FEHLEINGABE;

			 }

		CASE 4:

			 if (Orders)

			 {

				pop();

				CLS;

				if (Quit < MAX_ATTEMPT) Quit = MAX_ATTEMPT;

				Orders--;

			 }

			 else

			 {

				 FEHLEINGABE;

			 }

		CASE 5:

			 if (Orders)

			 {

				printf("AuftragsID bitte eingeben:\t");

				scanf("%i", &ID);

				if (ID > Orders)

				{

					printf("Dieser Auftrag existiert nicht.\n\n");

				}

				else

				{

					pop(ID);

				}

				CLS;

				if (Quit < MAX_ATTEMPT) Quit = MAX_ATTEMPT;

				Orders--;

			 }

			 else

			 {

				 FEHLEINGABE;

			 }

		CASE 6:

			 Quit = 0;

		DEFAULT:

			 FEHLEINGABE;

		}

	}

	return 0;

}

Das klappt auch mehr oder weniger...

Nur ist das Problem, dass er beim löschen (egal welchen Modus) immer den ersten Datensatz zuerst löscht und ich find den fehler nicht

jemand eine idee ?

[jasso]

:beagolisc :beagolisc

du kannst das doch sicher irgendwie zusammenfassen oder?

[TDM]

Was zusammenfassen ?

[Klotzkopp]

Nur ist das Problem, dass er beim löschen (egal welchen Modus) immer den ersten Datensatz zuerst löscht und ich find den fehler nicht

In pop führst du grundsätzlich immer free(head) aus.

Und ich rate dir nochmals, dir diese fiesen #define-Konstrukte abzugewöhnen. In C++ braucht man die nun wirklich nicht mehr.

[jasso]

na zum beispiel dein problem beschreiben oder den quellcode auf die für dein problem wesentlichen bestandteile zusammenfassen.

das hier

Nur ist das Problem, dass er beim löschen (egal welchen Modus) immer den ersten Datensatz zuerst löscht und ich find den fehler nicht

ist nicht so wirklich aufschlussreich.

Edit: ok ok unser Klotzkopp sieht das wohl anders.

[TDM]

In pop führst du grundsätzlich immer free(head) aus.

Und ich rate dir nochmals, dir diese fiesen #define-Konstrukte abzugewöhnen. In C++ braucht man die nun wirklich nicht mehr.

über das define lässt sich streiten

das free(head) ist nur in einem pop... und selbst da nur wenn das erste und das letzte element gleich sind (weil in dem Ausdruck if (head -> PNext) wird es (head) danach gleich wieder gesetzt :x)

Edit: im übrigen habe ich gerade rausgefunden, dass es nichts mit pop() zutun hat

[Klotzkopp]

über das define lässt sich streiten

Eigentlich nicht. Bis auf die merkwürdigen CASE und DEFAULT-Makros lässt sich das alles auch viel besser lösen. Du kannst übrigens ein typedef nicht mit #ifdef prüfen; da lassen dich die Makros schon im Stich.

Edit: im übrigen habe ich gerade rausgefunden, dass es nichts mit pop() zutun hat

Jedenfalls dekrementierst du in pop(Element_ID) die ID des letzten Eintrags nicht.

Wie wäre es denn, wenn du mal etwas genauer beschreibst, wie sich der Fehler reproduzieren lässt und wie er sich genau äußert?

[TDM]

das mit dem VL_Mode würd ich trotzdem über define machen, da sonst eine meldung von wegen neu-definition der funktionen kommt

aber lassen wir das

das problem besteht - so habe ich gerade rausgefunden - darin, dass alle werte überschrieben werden

Bsp:

Datensatz 1 mit Wert "Test1" hinzufügen:

Wert 1 = "Test1"

Datensatz 2 mit Wert "Test2" hinzufügen:

Wert 1 = "Test2"

Wert 2 = "Test2"

Datensatz 1 mit Wert "Test3" hinzufügen:

Wert 1 = "Test3"

Wert 2 = "Test3"

Wert 3 = "Test3"

[Klotzkopp]

das mit dem VL_Mode würd ich trotzdem über define machen, da sonst eine meldung von wegen neu-definition der funktionen kommt

Ich habe nicht gesagt, dass du die Makros ersatzlos streichen kannst. Aber auch das lässt sich besser lösen.

Aber zum eigentlichen Problem:

sNode::Elem ist ein Element, und das ist nur ein char*. Du reservierst aber nirgendwo Speicher für einen Text, du weist immer nur die Adresse des Puffers zu, den du zum Einlesen benutzt. Daher zeigen alle Einträge auf denselben Speicherbereich, und haben somit auch denselben Text.

[TDM]

ich habe jetzt folgendes probiert um den Speicher zu reservieren:

void push(Element data)

{

	Link tmp;

	tmp = new sNode;

	tmp -> Elem = new char[MAX_STRLEN];

	tmp -> Elem = data;												/* Daten eintragen */

	if (!head -> Elem)

	{

		head = tmp;

		tmp -> ID = 1;

		tmp -> PPrev = NULL;										/* Anfang auf NULL setzen */

	}

	else

	{

		tmp  -> PPrev = tail;										/* Verlinkung zum Vorherigen */

		tmp  -> ID    = (tmp -> PPrev -> ID + 1);

		tail -> PNext = tmp;										/* Verlinkung zum Nächsten */

	}

	tmp -> PNext = NULL;											/* Nächste vom Nächsten gibts nicht */

	tail = tmp;														/* Neues Ende */

}

das geht aber auch nicht so richtig O.o

[Klotzkopp]

tmp -> Elem = new char[MAX_STRLEN];

tmp -> Elem = data;

Mal ein Beispiel:

int a;


a = 5;

a = 2;[/code]

Wo ist die 5? Richtig, die ist weg, überschrieben durch die 2. Das gleiche machst du da. Du überschreibst den Zeiger auf den Speicherbereich, den du mit new geholt hast, mit der Adresse von data. Gleiches Verhalten wie vorher, aber jetzt noch ein zusätzliches Speicherleck.


Stichwort: strcpy. 


Übrigens solltest du new und malloc nicht mischen.

[TDM]

void push(Element data)

{

	Link tmp;

	tmp = new sNode;

	tmp -> Elem = new char[MAX_STRLEN];

	strcpy(tmp -> Elem, data);

	tmp -> Elem = data;												/* Daten eintragen */

	if (!head -> Elem)

	{

		head = tmp;

		tmp -> ID = 1;

		tmp -> PPrev = NULL;										/* Anfang auf NULL setzen */

	}

	else

	{

		tmp  -> PPrev = tail;										/* Verlinkung zum Vorherigen */

		tmp  -> ID    = (tmp -> PPrev -> ID + 1);

		tail -> PNext = tmp;										/* Verlinkung zum Nächsten */

	}

	tmp -> PNext = NULL;											/* Nächste vom Nächsten gibts nicht */

	tail = tmp;														/* Neues Ende */

}

dat funzt jetzt mehr oder weniger

btw: wo habe ich malloc mit new gemischt ?

hatte mir zwar erst überlegt ich mach #define NEW_ADDRESS new sNode aber hab das dann doch gelassen und lieber überall im text einzeln ersetzt...

und ja... ich habe auch free mit delete getauscht

naja egal... geht jetzt

danke

[Guybrush Threepwood]

hatte mir zwar erst überlegt ich mach #define NEW_ADDRESS new sNode aber hab das dann doch gelassen und lieber überall im text einzeln ersetzt...

Das ist nicht dein Ernst oder?

Darf ich dich fragen wie das bei dir in der Firma aussieht mit den Programmieren? Gibt es da noch andere die dir ein bisschen unter die Arme greifen bzw. sich deine Sachen mal angucken?

Das ist jetzt nicht böse gemeint aber das ist meiner Meinung nach aller übelster Programmierstil und ich persönlich würde mich hier nicht in so ein Programm reinlesen um einen Fehler zu finden. Glaub mir je schneller du dir sowas abgewöhnst um so besser für dich und für das Programm und um so schneller wirst du selber Fehler im Programm finden bzw. können andere das Programm lesen.

[Mephisto81]

hallo,

	tmp -> Elem = new char[MAX_STRLEN];

	strcpy(tmp -> Elem, data);

	tmp -> Elem = data;										

dat funzt jetzt mehr oder weniger

du weißt schon was du da machst oder? sieh dir nochmal klotzkopps posting an und vielleicht fällt dir an der letzten zeile was auf. Stichwort: Zeiger in C++. Dein Speicherleck ist immer noch da.

ansonsten muss ich meinem vorredner in gewissen aspekten absolut zustimmen.

gruss

mep

[TDM]

Das ist nicht dein Ernst oder?

Darf ich dich fragen wie das bei dir in der Firma aussieht mit den Programmieren? Gibt es da noch andere die dir ein bisschen unter die Arme greifen bzw. sich deine Sachen mal angucken?

Das ist jetzt nicht böse gemeint aber das ist meiner Meinung nach aller übelster Programmierstil und ich persönlich würde mich hier nicht in so ein Programm reinlesen um einen Fehler zu finden. Glaub mir je schneller du dir sowas abgewöhnst um so besser für dich und für das Programm und um so schneller wirst du selber Fehler im Programm finden bzw. können andere das Programm lesen.

Das war eigentlich als scherz gemeint weil ich am anfang das mit #define NEW_ADDRESSE (Link) malloc(sizeof(sNode)) gemacht hatte...

bin halt schreibfaul *g*

Mephisto:

sry, hatte mich verschrieben (bzw. verkopiert O.o) - die zeile tmp -> Elem = data; ist drausen