Das Löschen eines verknüpften Listenelements verursacht eine Endlosschleife - c, Schleifen, verkettete Liste, unendlich

Ich schreibe ein Programm, das eine Liste von ganzen Zahlen in der Eingabe nimmt und basierend auf der Ganzzahl die folgenden Operationen ausführt:

  1. Entfernen Sie den absoluten Wert, wenn der Wert in der Eingabe negativ ist

  2. Wenn die Zahl positiv und gerade ist, fügen Sie am Anfang der Liste hinzu

  3. Wenn die Zahl positiv und ungerade ist, fügen Sie sie am Ende der Liste hinzu

  4. Wenn die Zahl gleich Null ist, beenden Sie das Programm und drucken Sie die Liste.

Mein Problem ist mit der pop_el-Funktion, die eine Endlosschleife auf der Liste verursacht. Wenn ich also die Liste drucke, geht das Programm in eine Endlosschleife. Das ist mein Code:

#include <stdio.h>
#include <stdlib.h>

typedef struct ll_node_S * ll_node_ptr;
struct ll_node_S
{
int v;
ll_node_ptr next;
};
typedef struct ll_node_S ll_node;

ll_node_ptr push_tail(ll_node_ptr head, int v)
{
ll_node_ptr backup = head;
ll_node_ptr current = head;

while(current->next != NULL)
{
current = current->next;
}
current->next = (ll_node_ptr) malloc(sizeof(ll_node));
current->v = v;
return backup;
}

ll_node_ptr push_head(ll_node_ptr head, int v)
{
ll_node_ptr new_head = (ll_node_ptr)malloc(sizeof(ll_node));
new_head->v = v;
new_head->next = head;
return new_head;
}

ll_node_ptr pop_el(ll_node_ptr head, int el)
{
ll_node_ptr backup = head;
ll_node_ptr current = head;
ll_node_ptr previous = NULL;
int found = 0;

while(current != NULL && !found)
{
if(current->v == el)
{
if(previous == NULL)
{
backup = current->next;
free(current);
current = backup;
previous = current;
}
else
{
previous->next = current ->next;
free(current);
current = current->next;
}

found = 1;
}
else
{
previous = current;
current = current->next;
}
}
return backup;
}

void print(ll_node_ptr head)
{
ll_node_ptr current = head;
printf("%dn", head->v);
while(current->next != NULL)
{
current = current->next;
printf("%dn", current->v);
}
}

int isPair(int n)
{
return ((n % 2) == 0);
}

int main(int argc, char** argv)
{
int n = 1;
ll_node_ptr list = NULL;
while(n != 0)
{
scanf("%d", &n);

if(n < 0)
{
list = pop_el(list, -n);
}
else
{
if(isPair(n))
{
list = push_head(list, n);
}
else
{
list = push_tail(list, n);
}

}


}

print(list);
//should free the list
return 0;
}

und dies ist der Testfall (in Eingabe übergeben) Ich teste den Code gegen:

4
5
2
-4
-5
-3
9
2
0

was sollte die folgende Ausgabe erzeugen:

2
2
9

irgendwelche Hinweise?

Antworten:

1 für die Antwort № 1

Verschiedene Dinge,

Im pop_el,

1.If previous ist NULL dann müssen Sie nur Ihre bewegen head ptr zum nächsten Knoten. Damit es neu wird head.

if(previous == NULL)
{
backup = current->next;
free(current);
//current = backup;   ---> Not needed.
//previous = current; ---> Not needed.
break;  //            ---> No need of setting found flag. You can remove it
}

2.Wenn vorherige nicht ist NULL dann musst du nur auf das zeigen previous Knoten nächster ptr an die current Knoten der nächste Knoten.

else
{
previous->next = current ->next;
free(current);
//current = current->next; ---> Not needed.
break;  //            ---> No need of setting found flag. You can remove it
}

3.In push_tail Sie reservieren Speicher für current->next Knoten und in der nächsten Zeile, die Sie hinzufügen v zum aktuellen Knoten "s v. Das ist falsch. Überprüfen Sie Folgendes,

ll_node_ptr push_tail(ll_node_ptr head, int v)
{
ll_node_ptr backup = head;
ll_node_ptr current = head;
ll_node_ptr new = NULL; // Created new pointer
while(current->next != NULL)
{
current = current->next;
}
//current->next = (ll_node_ptr) malloc(sizeof(ll_node));
new = (ll_node_ptr) malloc(sizeof(ll_node));
//current->v = v;   ----> incorrect. new Value is actually replacing the old value.
new->v = v;       // New value is added in the newly created node.
new->next = NULL;
current->next = new;
return backup;
}

4. Sie können Ihre verbessern print Logik

void print(ll_node_ptr head)
{
ll_node_ptr current = head;
//printf("%dn", head->v);
while(current != NULL)
{
printf("%dn", current->v);
current = current->next;
}
}

0 für die Antwort № 2

Ihr unmittelbares Problem kann gelöst werden, indem Sie die push_tail() Funktion, wie von @ Vitek hingewiesen. Der ursprüngliche Code speicherte nicht nur den Wert im falschen Knoten, sondern konnte auch nicht setzen next Feld des neu zugeordneten Endknotens an NULL. Es gibt ein weiteres Problem in dieser Funktion: Sowohl die push_head() und das push_tail() Funktionen müssen einen Knotenzeiger erstellen und zurückgeben, wenn sie mit a aufgerufen werden NULL head Zeiger. Das Original push_head() Funktion tut dies, aber die push_tail() Funktion nicht. Wenn die erste vom Benutzer eingegebene Nummer ungerade ist, wird der erste Knoten über die Liste der Liste hinzugefügt push_tail() Funktion, die zu undefiniertem Verhalten führt (höchstwahrscheinlich ein Segmentierungsfehler).

Noch wichtiger: Sie sollten daran arbeiten, Ihren Code zu vereinfachen. Dies macht es einfacher zu schreiben und einfacher zu debuggen. Zum Beispiel, dein print() Funktion kann auf drei Zeilen reduziert werden:

void print(ll_node_ptr current)
{
while(current) {
printf("%dn", current->v);
current = current->next;
}
}

Es gibt mehrere Dinge, die getan werden können, um das zu verbessern push_tail() Funktion. Hier ist eine neue Version:

ll_node_ptr push_tail(ll_node_ptr head, int v)
{
ll_node_ptr current = head;
ll_node_ptr prev = NULL;
ll_node_ptr tail = malloc(sizeof(ll_node));

if (tail == NULL) {
fprintf(stderr, "Tail node allocation errorn");
exit(EXIT_FAILURE);
}

tail->v = v;
tail->next = NULL;

while (current) {
prev = current;
current = current->next;
}

if (head) {
prev->next = tail;
} else {
head = tail;
}

return head;
}

Sie müssen das Ergebnis von nicht umsetzen malloc(). Es gibt unterschiedliche Meinungen darüber, ob Sie dies tun sollten oder nicht, aber das Schreiben ohne den Cast vereinfacht den Code. Auch sollten Sie überprüfen, um sicherzustellen, dass malloc() hat den angeforderten Speicher erfolgreich zugewiesen. Dies ist besonders wichtig, wenn Sie stattdessen verwenden realloc(), weil dies zu Speicherlecks führen kann. Nachdem der neue Endknoten hier erstellt wurde, wird der v und next Felder werden sofort gesetzt.

Es scheint in der Regel einfacher zu sein, über eine verkettete Liste zu iterieren, indem man die current Knoten, anstatt auf die current->next Knoten. Verwendung der prev Knotenzeiger erleichtert dies. Die neue Funktion iteriert die Liste bis current == NULL, an welchem ​​Punkt prev zeigt auf den letzten Knoten der Liste. Für den Fall, dass head wurde als a übergeben NULL Zeiger (d. h. die Liste war leer, was passieren kann, wenn die erste vom Benutzer eingegebene Zahl ungerade ist), wird die Iterationsschleife übersprungen current wird initialisiert auf head. Der Code nach der Schleife setzt die next Feld des letzten Knotens, das auf den neu erstellten Endknoten zeigt, wenn der Funktion andernfalls eine nicht leere Liste zugewiesen wurde head wird zum neuen Endknoten. Endlich, head wird an die aufrufende Funktion zurückgegeben.

Es gibt viele Vereinfachungen, die auf die pop_el() Funktion und viel überflüssiger Code. Diese Funktion sollte wirklich komplett überarbeitet werden. Wo du hast:

previous->next = current ->next;
free(current);
current = current->next;

Du hast freed Der Speicher, auf den verwiesen wird current, dann versuchen Sie den Zeiger auf diesen Speicher zu dereferenzieren. Nach dem Anruf nach free()Du besitzt diese Erinnerung nicht mehr, und das ist es undefiniertes Verhalten. Stattdessen willst du:

current = previous->next;

Aber das ist nicht wirklich wichtig, denn found wird als nächstes eingestellt 1beendet die Schleife und die Funktion returns backup, ohne weitere Verwendungen von current. Sie sollten nur die obige Zuordnung zu entfernen current da es nicht benötigt wird.

Sie sollten diese Informationen verwenden können, um den Rest des Codes in Ihrem Programm zu verbessern. Es gibt andere Probleme, die Sie vielleicht betrachten möchten. Es ist generell eine schlechte Übung, typedef Zeiger - dies dient nur dazu, den Code zu verschleiernund erhöht die Wahrscheinlichkeit von Programmierfehlern. Die Spezifikation, die Sie zur Verfügung gestellt haben, macht nicht klar, was passieren soll, wenn mehrere Knoten denselben Wert haben, d. H., Sollten einer oder alle entfernt werden? Und was ist, wenn die erste vom Benutzer eingegebene Nummer ist 0?


-1 für die Antwort № 3

Vor allem würde ich vorschlagen, rekursive Funktionen beim Arbeiten mit Listen zu verwenden. Es ist viel einfacher zu debuggen und zu verstehen.

Ich glaube, ich habe ein Problem mit Ihrer push_tail-Funktion gefunden:

current->next = (ll_node_ptr) malloc(sizeof(ll_node));
current->v = v;

Sie ordnen Speicher für current-> next zu, weisen den Wert jedoch dem aktuellen Knoten zu. Das sollte sein

current->next = (ll_node_ptr) malloc(sizeof(ll_node));
current->next->v = v;
current->next->next = NULL;

Vielleicht hat das etwas mit deinem Problem zu tun.


Verwandte Fragen
Speisekarte