Keskustelu: Ohjelmointikysymykset: C++: Ohjelma kaatuu sattumanvaraisessa kohtaa

Kyseessä on "urani" paras projekti ja myöskin hankalin virhe. Tunteja on kulunut jo ihan tarpeeksi.

Ongelma on siis kaatuilu, joka vaikuttaa tapahtuvan melko sattumanvaraisessa kohtaa koodia. Koska Dev-cpp:n mukana tuleva debuggeri ei jostain syystä suostu toimimaan projektini kanssa, olen käyttänyt debuggaukseen cout << "teksti" << endl; tekniikkaa. Olen suoltanut ongelman lähistöllä olevan koodin täyteen noita tulostusrivejä ja ajellut toistaen virhettä(joka myöskin ilmenee melko sattumanvaraisesti, mutta se nyt ei ole mitenkään uutta). Kun tarkastelen ohjelman syötteitä kaatumisen jälkeen, viimeinen tulostunut rivi voi olla periaatteessa mikä vain ongelman lähistöllä oleva tuloste. Jotkin rivit ovat toki useammin viimeisinä, mutta ei niilläkään tunnu olevan mitään yhteistä.

Okei, on minulle tuntien ja päivien tuoksinnassa jäänyt sellainen kuva, että kaatumiset tapahtuvat new- ja delete -käskyjen yhteydessä. En nyt mene tästä kuitenkaan aivan takuuseen, koska en pääse testailemaan juuri tällähetkellä, mutta kyllähän se melko uskottavalta kuulostaisi. Olen tietenkin tarkistanut, että muistia vapautettaessa osoittimet osoittavat sinne, minne ne osoittivat varauksen jälkeenkin. Varmaa on myös se, että en yritä vapauttaa jo vapautettua muistia.

Muistit on tarkistettu memtestillä, eikä virheitä löytynyt :)

Mainitaan nyt vielä, että kaatuminen tapahtuu reitinhakualgoritmin(joka on ollut käytössä aiemmissa projekteissa ongelmitta) sisällä, vähän sitä ennen tai sitten vähän sen jälkeen.

Käyttis: winXP.

Tulostusdebuggauksen heikkous on, että tulostetutkaan rivit eivät aina ehdi ruudulle asti ennen kaatumista. Kannattaa siis tehdä erillinen debug-funktio, joka varmistaa tiedon perillemenon.

Dynaamista muistinvarausta kannattaa tietenkin varoa kovasti. Jos varaus ja vapautus toimivat, jäljelle jää oikeastaan enää viallinen osoitus. Voit vaikka ottaa vapautuksen kokonaan pois (debug-tarkoituksissa tietenkin), jos muistia riittää, tai voit koodata omat new- ja delete-operaattorit, jotka kierrättävät samoja muistialueita eivätkä koskaan vapauta mitään.

Jos graafiset debuggerit niskoittelevat, voit käyttää GDB:tä komentorivillä, (vaikka se Windowsissa onkin kurjaa, kun komentorivi on niin huono). Sillä kyllä selviää tarkka kaatumiskohta. Tarkista myös, että debuggausvaiheessa olet ottanut optimoinnit pois päältä ja että ylimääräinen debug-info on mukana. Nämä varmaan tiesitkin kyllä.

Mikä tarkalleen ottaen on se virheilmoitus, joka kaatumisesta tulee?

Minäkin arvelisin, että virhe ei sinänsä ole new- tai delete-käsky eikä pointtereissa välttämättä ole mitään vikaa. Todennäköisempää on, että jossain kirjoitellaan varattujen muistialueiden yli, jolloin muistinhallinnan sisäinen kirjanpito menee sekaisin tai töhritään muita pointtereita. Silloin virhe ilmenee nimenomaan new- tai delete-operaatiossa.

Onko koodissa siis C-mäisyyksiä, kuten pakotettuja casteja, esmes memcpy-, memset- tai strcpy-kutsuja, tai esitelläänkö muuttujat aina funktioiden alussa ja alustetaanko ne vasta myöhemmin? Noista lähtisin etsimään ensiksi, kun en koodista muuten mitään tiedä. Mitä enemmän koodissa on C++:ssaa, sitä vähemmän tällaisia virheitä tapaa tulla.

Ongelma todellakin oli sisäisen muistinhallinnan töhriintyminen.

for(int i = start; i >= 0; --i) {
   point[i-1].x = x;
   point[i-1].y = y;
}

Olihan tuo tullut syynättyä, mutta ei siinä mielessä, että tulisi sörkittyä taulukon "etupuolelle".

Kiitokset vinkeistä!

Hyvä että selvisi. Nuo ovat joskus aika hankalia.

Onko muuten joku erityinen syy käydä tuota taulukkoa läpi takaperin? Kuten tästäkin näkyy, tällaiset jutut voivat vähän vaikuttaa "työturvallisuuteen". Tehokkuuden kannalta se ei liene ainakaan parempi kuin perinteinen

for(int i = 0; i < num; ++i) {

mutta enpä nyt lähde mittailemaan.

Arvaanko oikein, että koodin perusrakenne on osapuilleen tämä:

Point* point = new Point[count];
for(int i = num; i > 0; --i) {
   point[i-1].x = x;
   point[i-1].y = y;
}
delete [] point;

Jos miettii koodissa olleen bugin anatomiaa, niin turvallisempia koodauskäytäntöjä saattaisi kyllä löytyä. Aivan ensiksi homman voisi ajatella näin:

Point* point = new Point[count];
for(int i = 0; i < num; ++i) {
   point[i].x = x;
   point[i].y = y;
}
delete [] point;

Mutta kun C++:aa koodaa, niin voisihan tuo mennä näinkin:

Point* point = new Point[count];
fill_n(point, num, Point(x, y));
delete [] point;

Ja suoraa dynaamista muistinhallintaa kannattaa välttää, joten:

vector<Point> points(count);
fill_n(points.begin(), num, Point(x, y));

Vähemmän ja turvallisempaa koodia - vähemmän bugeja! Eikä suorituskyky ole ainakaan heikompi. Yleensä. :-)

Arvasti oikein koodin perusrakenteen osalta.

Vika siis löytyi useaan otteeseen käytetystä reitinhaku algoritmista, mikä sinänsä ihmetyttää minua suuresti, koska vastaavanlaista ongelmaa ei ole aiemmin ilmaantunut. Tässä tapauksessa taulukko on käytävä läpi päinvastaisessa järjestyksessä, jottei sitä tarvitse kääntää jälkikäteen tai laskeskella sijoitusvaiheessa arraySize - i laskuja.

Koodi on peräisin ajalta, jolloin koodasin c/c++:aa. En viitsinyt turhaan kirjoittaa uutta algoritmia, koska minulla oli jo "toimiva".

Gaxx kirjoitti:

Tässä tapauksessa taulukko on käytävä läpi päinvastaisessa järjestyksessä, jottei sitä tarvitse kääntää jälkikäteen tai laskeskella sijoitusvaiheessa arraySize - i laskuja.

Ainakaan esittämässäsi koodissa suunnalla ei ole merkitystä. Toisaalta esimerkkikoodisi on niin pelkistetty, että varmaankin silmukassa on oikeasti jotain järkeä.

Jos silmukoiden indeksointi takaperin tuottaa päänvaivaa, yksi konsti on kahden muuttujan käyttö: yhden laskurina ja toisen indeksinä.

for (lask = 0, i = N - 1; lask < N; ++lask, --i) {
  // N-1, N-2, ..., 1, 0
}

Toisaalta itseltäni tulee jo selkäytimestä tämä hieman obskyyrimpi rakenne, joka perustuu siihen, että post-decrement-operaattori (i--) palauttaa arvon ennen muutosta:

for (i = N; i--;) {
  // N-1, N-2, ..., 1, 0
}

Erityisesti suosittelen, ettet koskaan sorru tekemään silmukkaa, jossa käytät indeksinä vain i-1:tä kuten esimerkissäsi. Helpommalla pääsee lisäämällä -1:n silmukan lähtöarvoon ja rajaan. :)

Metabolix kirjoitti:

Ainakaan esittämässäsi koodissa suunnalla ei ole merkitystä. Toisaalta esimerkkikoodisi on niin pelkistetty, että varmaankin silmukassa on oikeasti jotain järkeä.

Kyllä, silmukasta löytyy paljon koodia.

Metabolix kirjoitti:

Jos silmukoiden indeksointi takaperin tuottaa päänvaivaa, yksi konsti on kahden muuttujan käyttö: yhden laskurina ja toisen indeksinä.

for (lask = 0, i = N - 1; lask < N; ++lask, --i) {
  // N-1, N-2, ..., 1, 0
}

Eipä tuo päänvaivaa tuota, virhe muiden joukossa. Minusta kahden muuttujan käyttö lisää virheen riskiä, kun täytyy ajatella kahta muuttujaa yhtäaikaa. Ei tosin ole kokemusta.

Metabolix kirjoitti:

Erityisesti suosittelen, ettet koskaan sorru tekemään silmukkaa, jossa käytät indeksinä vain i-1:tä kuten esimerkissäsi. Helpommalla pääsee lisäämällä -1:n silmukan lähtöarvoon ja rajaan. :)

Joo, toki korjasin tuon samalla kuntoon ihan nopeussyistäkin(vaikka nimellinen onkin).