Keskustelu: Ohjelmointikysymykset: C++, PL/I: Osoittimien deletointi? C++

Koodi toimii muuten mutta en osaa deletoida osoittimia oikein.

#include <cstdlib>
#include <iostream>
#include <string>
using namespace std;


struct Tapahtuma {
	string viesti;
	Tapahtuma* seuraava;
};

int main() {

	Tapahtuma* eka;
	Tapahtuma* temp;
	string luettu;
	int maara = 0;

	eka = new Tapahtuma;
	temp = eka;

	while(maara < 10) {
		cout << "Kirjoita viesti: ";
		cin >> luettu;

		temp->viesti = luettu;

		temp->seuraava = new Tapahtuma;
		temp = temp->seuraava;
		++maara;
	}

	//deletointi
	for (Tapahtuma* t = eka; t != 0; t = t->seuraava) {
   		delete t;
   	  	t = 0;
	}
}

Miten deletointia pitäisi muuttaa?

For-silmukkasi tekee vertailun tuhottuun alkioon. Näin ollen se lopettaa heti ensimmäisen alkion vapauttamisen jälkeen. Muuta se niin että haet seuraavan alkion talteen _ennen_ kuin tuhoat siihen osoittavan alkion.

Edit. Mikko kerkesikin jo vastata. Piti korjata, että vertailuun asti tuskin koskaan edes päästään, koska silmukassa osoitin ensin nollataan ja sitten sen nollatun osoittimen kautta yritetään hakea seuraavaa. Sama ohje korjaamiseen pätee silti.

Putkalainen kirjoitti:

	//deletointi
	for (Tapahtuma* t = eka; t != 0; t = t->seuraava) {
   		delete t;
   	  	t = 0;
	}

Ensinnäkin poistettaessa asetat nykyisen tietueen osoittimen nollaksi, jolloin kun yrität siirtyä seuraavaan tietueeseen yritätkin lukea nollaosoitteessa olevan tietueen tietoja, jolloin ohjelma kaatuu.

Vaikka et muuttaisikaan osoitinta nollaksi, on parantamista vieläkin, sillä ainakaan minun muistikuvani mukaan delete:llä vapautettuun muistialueeseen ei missään nimessä saa enää viitata (tässä on varmaan jotain erityisiä yksityiskohtia, joita joku paremmin tietävä voi kertoa).

Ota siis talteen osoitin seuraavaan tietueeseen ennen kuin poistat nykyisen tietueen, ja käytä sitten tätä osoitinta siirtyäksesi eteenpäin.

Tapahtuma* t = eka;

while(t != 0)
{
	Tapahtuma* tmp = t->seuraava;

	delete t;

	t = tmp;
}

EDIT: Torgo olikin sitten nopeampi :)

mikkop92 kirjoitti:

Tässä on varmaan jotain erityisiä yksityiskohtia, joita joku paremmin tietävä voi kertoa.

Ei siinä ole mitään yksityiskohtia. Muisti on vapautettu, sitä ei saa käyttää. Joissain järjestelmissä koodi voi toki tuurilla silti toimia.

Koodissa on toinenkin olennainen virhe: mikään ei takaa, että juuri luodun tapahtuman seuraava olisi 0, joten listan viimeisen jäsenen jälkeen päädytään vapauttamaan jo lähtökohtaisesti virheellisiä osoittimia. Osoitin seuraavaan jäseneen pitää siis itse asettaa nollaksi new-operaattorin jälkeen tai viisaammin Tapahtuma-luokan muodostimessa.

#include <cstdlib>
#include <iostream>
#include <string>
using namespace std;

struct Tapahtuma {
    string viesti;
    Tapahtuma* seuraava;
};

int main() {

    Tapahtuma* eka;
    Tapahtuma* temp;
    string luettu;
    int maara = 0;

    eka = new Tapahtuma;
    temp = eka;

    while(maara < 10) {
        cout << "Kirjoita viesti: ";
        cin >> luettu;

        temp->viesti = luettu;

        temp->seuraava = new Tapahtuma;
        temp = temp->seuraava;
        ++maara;
    }

    //deletointi
    Tapahtuma* t = eka;

    while(t != 0) {
        Tapahtuma* tmp = t->seuraava;
        delete t;
        t = tmp;
    }
}

Antaa edelleen "segmentation fault"

Auttaisikohan tuo Metabolixin esittämä toisen virheen korjaaminen. Eli:

struct Tapahtuma {
    Tapahtuma(): seuraava(0) { }
    string viesti;
    Tapahtuma* seuraava;
};

Lisäksi listassasi on ylimääräinen alkio, mikä ei kuitenkaan menoa haittaa.

Toivottavasti huomasit viestini yllä. Jatkan kuitenkin neuvomalla yksinkertaista ohjelman debuggausta:

• Tulosta, mitä on luotu.

  // Alussa:
  std::cout << "new: " << eka << std::endl;
  // Lukusilmukan lopussa:
  std::cout << "new: " << temp << std::endl;

• Tulosta, mitä yritetään tuhota.

  // Tuhoamissilmukan alussa:
  std::cout << "delete: " << t << std::endl;

Vertaa osoittimia. Näyttävätkö samalta? (Testausta varten kannattaa vähentää kierrosten määrää.)

Itse toteuttaisin linkityn listan hiukan erilailla, johtuen ihan käytännön syistä:

tyhjä lista näyttäisi tältä:

head-> tail -> tail

Eli kaksi dummy nodea (head ja tail) ja tail noden next-osoitin osoittaa itseensä NULL:in sijasta.

Ja jos kyseessä on esimerkiksi lista, mihin tallennetaan lukuja suuruusjärjestykseen, niin tällöin tail nodesta voi tehdä vahdin (sentinel) laittamalla siihen suurimman mahdollisen sallitun lukuarvon. Nyt listaan lisätessä vertailusilmukka pysähtyy viimeistään aina ennen tail nodea, eikä ole tarvetta erikseen testata, ollaanko listan lopussa.

Ja jos ei joku vielä itse huomannut, niin listasta saa vieläkin tehokkaamman muuttamalla listasta ympyrän mallisen yhdistämällä head ja tail nodet yhdeksi head/tail nodeksi.

jalski kirjoitti:

Itse toteuttaisin linkityn listan hiukan erilailla, johtuen ihan käytännön syistä:

tyhjä lista näyttäisi tältä:

head-> tail -> tail

Eli kaksi dummy nodea (head ja tail) ja tail noden next-osoitin osoittaa itseensä NULL:in sijasta.

En tiedä, tajusiko ketjun aloittaja mitä tarkoitin yllläolevalla, mutta laitetaanpas esimerkki näkyville.

Koska on opettavaisempaa olla antamatta valmista koodia, niin kirjoitetaankin se jokaisen "vanhan liiton ohjelmoijan" lempiohjelmointikielellä: ;-)

Tunnistaako kukaan muuten?

linkedlist: package;


   dcl 1 node based (nodeptr),
	2 value fixed bin (31),
	2 next ptr;

   dcl nodeptr ptr static;
   dcl headptr ptr static;
   dcl tailptr ptr static;

   dcl (allocate, plifree) builtin;


 main:proc options(main);
   put skip list ('Creating empty list...');
   call create_list;
   put skip list ('Test by adding some nodes to head of the list...');
   call add_node(10);
   put skip list ('...10');
   call add_node(20);
   put skip list ('     ...20');
   call add_node(30);
   put skip list ('          ...30');
   call add_node(40);
   put skip list ('               ...40');
   put skip;
   put skip list ('Now, let''s print the contents of the list:');
   call print_list();
   put skip list ('All done, destroy the list and exit.');
   call destroy_list;
 end main;


 create_list: proc;
   allocate node;
   headptr = nodeptr;
   allocate node;
   tailptr = nodeptr;

   headptr->next = tailptr;
   tailptr->next = tailptr;
 end create_list;


 destroy_list: proc;
   nodeptr = headptr->next;

   do while (nodeptr ¬= nodeptr->next);
      headptr->next = nodeptr->next->next;
      call delete_node(nodeptr);
      nodeptr = headptr->next;
   end;

   call delete_node(headptr);
   call delete_node(tailptr);
 end destroy_list;


 add_node: proc(v);
   dcl v fixed bin (31);

   allocate node;
   nodeptr->value = v;
   nodeptr->next = headptr->next;
   headptr->next = nodeptr;
 end add_node;


 delete_node: proc(nodeptr);
   dcl nodeptr ptr;

   call plifree(nodeptr);
 end delete_node;


 print_list: proc;
   nodeptr = headptr->next;

   do while (nodeptr ¬= nodeptr->next);
      put skip list ('->' || nodeptr->value);
      nodeptr = nodeptr->next;
   end;
 end print_list;


end linkedlist;