Unbekanntes Speicherzugriffsproblem Structs in Liste

[tomi89]

Hallo,
ich möchte eine einfach-verkettete Liste erstellen und als Knoten geometrische Objekte darin speichern.

Die Struktur fuer die Objekte sieht folgendermaßen aus:

struct listElem {
	struct{              /* geschachtelte Struktur */
        float area;
        float hoehe;
        float breite;
        float laenge;
        float radius;
        char kuerzel;
       }flaeche;
	struct listElem *next;
};
typedef struct listElem *LISTELPTR;

LISTELPTR ListenPtr;                /* auf den Anfang der Liste */
LISTELPTR listenPtr = NULL;	/* auf Listenanfang */

So, nun erstelle ich mir ueber eine Funktion erst einmal einen neuen Knoten und uebergebe die eingegebenen Werte, damit ich den Knoten gleich belege:

/* Platz reservieren, mit val besetzen, Zeiger liefern */
LISTELPTR machListEl(float area,float hoehe,float breite,float laenge,float radius,char kuerzel)
{
	LISTELPTR newptr;					/* lokal */

	if((newptr=(LISTELPTR)malloc(sizeof(struct listElem))) != NULL);{
        newptr->flaeche.area = area;
		newptr->flaeche.hoehe = hoehe;
		newptr->flaeche.breite = breite;
		newptr->flaeche.laenge = laenge;
		newptr->flaeche.radius = radius;
		newptr->flaeche.kuerzel = kuerzel;
		newptr->next = NULL;	     	/* nicht "next" wählen, sonst runtime-error */
	}
	return newptr;                      /* Null, falls erfolglos */
}//machListEl

Bauchschmerzen macht mir die Funktion, die die Knoten an die Liste kettet:

LISTELPTR inListeAnfuegen(float area,float hoehe,float breite,float laenge,float radius,char kuerzel)
{
   LISTELPTR static listenPtr = NULL;	/* auf Listenanfang */
   LISTELPTR newElptr,				    /* auf neues Element	*/
	         hilfptr;                   /* nur zu Hilfszwecken */

    newElptr = machListEl(area, hoehe, breite, laenge, radius, kuerzel);   /* ein neues Listenelement erzeugen,
                                               d.h. Platz reservieren und anschließend
                                               die Werte in das struct schreiben */
    if(listenPtr == NULL) {               /* Liste  ist noch leer */
        listenPtr = newElptr;
    }
    else
    {   // Suche des letzten Listenelementes
        printf("\n\nlistenPtr NOT NULL\n\n");
        hilfptr = listenPtr;
        while(hilfptr != NULL )
        {
            hilfptr = hilfptr->next;	/* Zeiger durchiterieren */
        }
        // Stelle gefunden: hilfptr zeigt auf NULL, also Ende der Liste
        [u]hilfptr->next = newElptr;    /* an letzter Stelle einfügen */[/u]
    }
   return listenPtr;
}

Ich habe durch printf()-Debugging herausgefunden, dass die unterstrichene Stelle die Probleme bereitet und gcc -lm mir ein Speicherzugriffsproblem schmeißt, ich erkenne den Fehler aber absolut nicht..

Der QC ist ziemlich lang (466 Zeilen) und ich wollte mich hier nur aufs Wesentliche beschraenken, kann ihn gerne aber anhaengen oder hier reinquoten, wenn er vollstaendig gebraucht wird.

Ich hoffe mir kann jemand erklaeren was ich da falsch mache.
Danke schonmal fuers Lesen,
Tomi.

[seldon]

Hier:

// Stelle gefunden: hilfptr zeigt auf NULL, also Ende der Liste
        hilfptr->next = newElptr; /* an letzter Stelle einfügen */

zeigt hilfptr in der Tat auf NULL. hilfptr->next kann dementsprechend nicht funktionieren.

Versuch's mal mit

while(hilfptr->next != NULL )

ein paar Zeilen darüber, dann zeigt hilftptr am Ende nicht auf NULL, sondern auf das letzte Listenelement (dessen next-Zeiger nach NULL zeigt).

[CStoll]

Du läufst in der Schleife (Zeile 17 bis 20) zu weit - die wird erst beendet, wenn hilfsptr auf NULL zeigt. und dahinter versuchst du dann, diesen NULL-Zeiger zu dereferenzieren.

PS: Dir ist schon klar, daß du mit diesem Code nur eine einzige Liste verwalten kannst? Und wenn die anderen Verwaltungsfunktionen (ausListeLoeschen etc) auch einen LISTELPTR static listenPtr = NULL; haben, wird's merkwürdig. Du solltest die verwaltete Liste lieber als (Referenz-)Parameter an diese Funktionen übergeben.

PPS: Wenn sich Variablennamen nur in Groß- und Kleinschreibung unterscheiden (und dann noch teilweise durch lokale Variablen überdeckt werden), bekommst du schnell Probleme beim Debugging

[Wutz]

tomi89 schrieb:

LISTELPTR ListenPtr;                /* auf den Anfang der Liste */
LISTELPTR listenPtr = NULL;	/* auf Listenanfang */

Wenn du mit lokal statischen Listen arbeitest, ist dies hier natürlich Unsinn.

tomi89 schrieb:

if((newptr=(LISTELPTR)malloc(sizeof(struct listElem))) != NULL);{
                                                                   ^ was soll das denn?

[tomi89]

Oha, das ging ja ziemlich schnell
Vor lauter Codeblindheit nicht gesehen, dass ich den Nullzeiger benutze ~~
Jetzt funktioniert die Geschichte auch!

CStoll schrieb:

PS: Dir ist schon klar, daß du mit diesem Code nur eine einzige Liste verwalten kannst? Und wenn die anderen Verwaltungsfunktionen (ausListeLoeschen etc) auch einen LISTELPTR static listenPtr = NULL; haben, wird's merkwürdig. Du solltest die verwaltete Liste lieber als (Referenz-)Parameter an diese Funktionen übergeben.

Ja Aufgabenstellung war, dass ich nur eine Liste benutzen darf.
Das mit der Zeigerübergabe ist natürlich ein besserer Weg, da hast du Recht, das werde ich gleich noch ändern.

CStoll schrieb:

PPS: Wenn sich Variablennamen nur in Groß- und Kleinschreibung unterscheiden (und dann noch teilweise durch lokale Variablen überdeckt werden), bekommst du schnell Probleme beim Debugging

Hast du auch vollkommen Recht, ich hab meinen Code erst ganz nach meinem eigenen Stil programmiert aber, weil ich den Fehler nicht gefunden habe, habe ich angefangen Vorlagen, die vorgegeben waren und funktionieren sollten einzubauen.
Bin mit den Variablennamen auch nicht zufrieden, wird noch geändert, wollte nur erstmal, dass das Ganze läuft.

Wutz schrieb:

tomi89 schrieb:

LISTELPTR ListenPtr;                /* auf den Anfang der Liste */
LISTELPTR listenPtr = NULL;	/* auf Listenanfang */

Wenn du mit lokal statischen Listen arbeitest, ist dies hier natürlich Unsinn.

Da hast du Recht ich benutze diese globalen Variablen aber in der Tat zur Uebergabe fuer die Print-Funktion, die mir den Ganzen Spaß auf der Konsole ausgibt.
Mein Professor wollte wohl nur von uns wissen, was static bedeutet und hat es an dieser Stelle so gedeichselt, dass der "Add"-Funktion nichts uebergeben wird, sondern mit static gearbeitet werden soll.

tomi89 schrieb:

Wutz schrieb:

if((newptr=(LISTELPTR)malloc(sizeof(struct listElem))) != NULL);{
                                                                   ^ was soll das denn?

Die Idee war:
Ich allokiere Speicher in der Groesse meiner Struktur fuer jeden Knoten.
und malloc() gibt den Zeiger auf diesen Speicherbereich zurueck, also pruefe ich auf NULL, damit ich weiss ob Speicher allokiert werden konnte (Zeiger wird zurueckgegeben) oder nicht (NULL-Zeiger wird zurueckgegeben).

€dit: Achso ich Horst hab natuerlich den Syntaxfehler uebersehen^^

Wollte eure Anmerkungen kommentieren, um noch Verständnisprobleme meinerseits aufzudecken.
Und ausserdem weil ihr mir so schnell und ausfuehrlich geholfen habt.

Danke nochmal!

PS: koennt gerne noch weiter nachhaken falls Bedarf an geeigneter Stelle besteht.

[CStoll]

Wutz hat imo keine Probleme mit der Anweisung an sich, nur mit dem Semikolon hinter dem if(). Das stellt nämlich eine eigene (nop-)Anweisung dar, die bei erfüllter Bedingung ausgeführt wird.
Im Ergebnis machst du gar nichts, wenn die Speicher-Anforderung schiefgegangen ist, und dein Initialisierungs-Code wird trotzdem ausgeführt, auch wenn es schiefgelaufen ist.

[Wutz]

tomi89 schrieb:

if((newptr=(LISTELPTR)malloc(sizeof(struct listElem))) != NULL);{
                                                                   ^ was soll das denn?

Die Idee war:

Du hast nicht verstanden, das ';' gehört da nicht hin und ist vollkommen falsch wenn auch syntaktisch korrekt.

Außerdem sollte sowas einfacher gehen, z.B.

LISTELPTR machListEl(float area,float hoehe,float breite,float laenge,float radius,char kuerzel)
{
  struct listElem tmp = {area,hoehe,breite,laenge,radius,kuerzel};
  LISTELPTR newptr = malloc( sizeof*newptr );
  if( newptr )
    *newptr = tmp;
  return newptr;
}

[tomi89]

Das mit dem Syntax-Fehler hatte ich oben editiert in meinem Beitrag.

Ich muss den Thread nochmal aufgreifen, ich bin nun dabei das Array, welches die structs enthaelt aus denen ich meine Liste baue zu sortieren.
Zu Übungszwecken nutze ich Insertionsort, mit Bubblesort und Quicksort hatte ich keine Probleme.

Ich bin den Code schon auf Papier durchgegangen und eigentlich muesste das Array korrekt aufsteigend sortiert werden.
Aber stattdessen bekomme ich diese Ergebnisse:
Zum Beispiel: array[6] = {3,1,5,2,6,4}
Ausgabe nach sortieren: 3,3,5,5,6,6

Ich weiß nicht wie das zustande kommen mag..
Hier der Code von InsertionSort:

void insertionSort(struct Figuren *array, int length)
{
    int i, j;
    struct Figuren *current, *swap;

   for(i=1; i<length; i++)
   {
      current = &array[i];
      j = i;
      while((j > 0) && (array[j-1].area > current->area))
      {
         array[j] = array[j-1];
         j = j - 1;
      }
      array[j] = *current;
   }
}

Meine Struct haelt ein float-Wert area, auf den ich pruefe und nach diesem moechte ich aufsteigend sortieren.

[CStoll]

Du solltest dir das Element selber merken, das du neu eingruppieren willst, nicht nur seine Position im ursprünglichen Array (dort wird es nämlich gleich beim ersten Durchlauf der inneren Schleife überschrieben).

[tomi89]

Wie stelle ich das an? Hab jetzt mehrere Sachen probiert, aber nichts funktioniert.
Als ich versucht habe deinen Rat umzusetzten gabs wieder Zugriffsprobleme, ich stell mich wahrscheinlich doof an, aber ich blicke da nicht durch.

void insertionSort(struct Figuren *array, int length)
{
    int i, j;
    struct Figuren *current;
    struct Figuren temp;

   for(i=1; i<length; i++)
   {
      current = &array[i];
      temp = array[i];
      j = i;
      while((j > 0) && (array[j-1].area > current->area))
      {
         array[j] = array[j-1];
         j = j - 1;
      }
      array[j] = temp;
   }
}

Wieso überschrieben wird ist mir nun klar, aber ich finde keine Lösung...

[CStoll]

Wenn du jetzt noch in der Schleife mit temp.area vergleichst, sollte es eigentlich richtig sein. (und den Zeiger current kannst du dann weglassen)