[C++] Linux / Mysql - segmentatie fout

[denpries]

Beste mensen,

Al enige tijd trad er na X uur (uur of 10 maar wel altijd na even veel tijd) een segmentatie fout op in een zelf gemaakt programma. Best irritant. Valgrind werd ik niet veel wijzer van dus ik ben maar begonnen door secties uit de code aan en uit te zetten en het programma steeds weer uit te proberen. Inmiddels ben ik er achter dat dit stukje code in ieder geval een oorzaak is. Zonder deze code actief geen segmentatie fout.

Per toeval stuitte ik op het gegeven dat ik geen mysql_free_result(confres) gebruikte. Dit lijkt mij op dit moment de oorzaak maar ik heb het nog niet getest. Maar hoe komt dit eigenlijk? Ik ging er vanuit dan confres out of scope ging en dus vanzelf de allocatie verloor? Of verliest alleen de pointer zijn allocatie, maar de gealloceerde data pas na de free result?

Verder gebruik ik nog lret = *mysql_fetch_row(confres); waar lret dus een char*. Echter MYSQL_ROW, de output vanmysql_fetch_row(confres) is volgens mij een char**. Kan dit ook problemen opleveren? De database levert overigens maar 1 waarde van 1 cijfer lang.

Ik geloof overigens dat ik het beste sowieso chars kan vermijden maar waar mogelijk std::strings te gebruiken, of is dat niet zo?

Onderstaande code komt uit een msql_connectie class waarin conn de MYSQL connectie is.

code:

double msql_conn::GetSetting(int settingID) {
    
        char * lret;

        double code;
        std::stringstream ss; //create a stringstream
        std::string querytext;
        ss << settingID; //add number to the stream

        querytext = std::string("SELECT settings_actief FROM settings 
           WHERE settings_id= ") + std::string(ss.str());
        
        if (mysql_query(conn, querytext.c_str())) {
            return (-1.0);
        } else {
            MYSQL_RES *confres = mysql_store_result(conn);
            int totalrows = mysql_num_rows(confres);
            int numfields = mysql_num_fields(confres);

            if (totalrows != 0 && numfields != 0) {

                lret = *mysql_fetch_row(confres);//<-- moet met MYSQL_ROW? (Oorzaak?)
                code = atof(lret);
                mysql_free_result(confres);//<-- dit had ik nog niet, oorzaak?
                return (code);

            } else {
                mysql_free_result(confres);//<-- dit had ik nog niet, oorzaak?
                return (-1.0);
            }

        }
    
}

[Matis]

denpries schreef op donderdag 23 mei 2013 @ 09:43:
Valgrind werd ik niet veel wijzer van

Ik denk dat daar de crux van het probleem zit.

Ik gebruik valgrind altijd met de volgende argumenten en er komt in 99% van de gevallen exact uit waar de pijnpunten liggen.

valgrind --tool=memcheck --verbose --leak-check=yes --show-reachable=yes --track-fds=yes --log-file="valgrind_output_`date +%Y-%m-%d_%H_%M_%S`_log.txt" ./DE_NAAM_VAN_JE_APP_HIER

Daarnaast snap ik niet waarom je op meerdere plekken returned en waarom je zowel in de if, als de else mysql_free_result(confres); hebt opgenomen. Ik zou dat stukje sws herschrijven voor de leesbaarheid.

Daarnaast ontwikkel ik alleen maar in GNU C99. Maar daar moet je expliciet alles free-en. De pointer valt dan misschien wel buiten de scope, maar de gealloceerd data niet.

[ Voor 10% gewijzigd door Matis op 23-05-2013 10:00 ]

[Zoijar]

denpries schreef op donderdag 23 mei 2013 @ 09:43:
Ik ging er vanuit dan confres out of scope ging en dus vanzelf de allocatie verloor? Of verliest alleen de pointer zijn allocatie, maar de gealloceerde data pas na de free result?

Ja, een pointer wordt helemaal geen destructor voor aangeroepen, dus er wordt niets opgeruimd.

Sowieso is dit slecht ontwerp: als bijvoorbeeld je mysql_fetch_row throwed (zal niet gebeuren maar voor het idee) lek je alsnog. Bovendien heb je grote kans dat je toch een keer een free vergeet.

Dit is een van de moeilijke en belangrijke dingen van C++. Zoek maar eens op RAII (resource acquisition is initialization). Normaal los je dit op met een std::shared_ptr die de pointer wrapped en freed. Maar je gebruikt ook een C API in C++. Misschien kan je beter eens naar dit soort oplossingen kijken: http://tangentsoft.net/mysql++/

[denpries]

Zoijar schreef op donderdag 23 mei 2013 @ 12:45:
[...]

Sowieso is dit slecht ontwerp: als bijvoorbeeld je mysql_fetch_row throwed (zal niet gebeuren maar voor het idee) lek je alsnog. Bovendien heb je grote kans dat je toch een keer een free vergeet.

Dit snap ik niet helemaal. Wat is throwen? En waarom is er dan alsnog een leak?

Hoe zou ik deze functie bijvoorbeeld het best kunnen herschrijven dan? Als je een voorbeeld hebt kan ik andere functies ook langsgaan om ze op eenzelfde manier in te richten.

[Zoijar]

Ja, dat is een beetje lastig zo snel uit te leggen zonder dat je veel over C++ moet leren. Als je op "C++ RAII" zoekt vind je wel artikelen daarover. Ik raad je wel aan daar over te lezen, want het is een van de belangrijkste dingen om goed te snappen in C++.

Het idee achter deze manier van programmeren is dat je eigenlijk vrijwel nooit expliciet een 'free' voor een resource aanroept. Je laat dat automatisch doen door een destructor van een wrapper als de resource wrapper uit scope gaat. In C++11 zou je het zo kunnen doen (hoewel ik vrees dat dit een beetje "beyond the scope of this post" is...):

C++:

std::shared_ptr<MYSQL_RES> confres(
   mysql_store_result(conn),
   [](MYSQL_RES* p) {mysql_free_result(p);}
);

Maar misschien is het makkelijker om daar een ander zich druk over te laten maken en een C++ mysql library te gebruiken zoals die link boven.

[denpries]

Waar zit em precies de voordelen in van een c++ versie van die library boven de c? Je kunt in principe beiden gebruiken natuurlijk, maar waar zit het echte voordeel?

[Zoijar]

denpries schreef op donderdag 23 mei 2013 @ 20:32:
Waar zit em precies de voordelen in van een c++ versie van die library boven de c? Je kunt in principe beiden gebruiken natuurlijk, maar waar zit het echte voordeel?

Dat een (goede) C++ library de resource management voor je doet, of althans versimpelt. Verder interfacet het vaak direct met STL containers.

Zoiets werkt toch veel lekkerder:

C++:

vector<stock> v;
query << "SELECT * FROM stock";
query.storein(v);
for (vector<stock>::iterator it = v.begin(); it != v.end(); ++it) {
  cout << "Price: " << it->price << endl;
}