Opravujeme chyby v softwaru: inotify-tools

Příběh o tom, jak probíhá hledání a opravování chyby v autorovi dosud neznámém kódu, vypráví článek Opravujeme chyby v softwaru: inotify-tools. Krok za krokem stáhneme zdrojové kódy, přeložíme program a najdeme chybu – seznámíme se se základními postupy a nástroji pro diagnostiku chyb (verzovací systém, IDE, GNU Debugger, Kompare).

5.10.2015 12:14 | xkucf03 | Zajímavý článek

Komentáře

[Boris Dušek]

Pěkné

[Dadam]

S tou předčasnou optimalizací je trochu vedle. Jak je vidět z

for ( i = 0; i < strlen(string); ++i ) {

autoři si s optimalizacemi moc hlavu nedělali. Je to o vyhnutí se dynamické alokaci (o 4kB poli v lokální proměnné nemluvě).

Na githubu mají dobrou opravu:

Do not double escape names. Fixes #36

[xkucf03]

S tou předčasnou optimalizací je trochu vedle. Jak je vidět z

Asi optimalizovali na obsazenou paměť ne na výkon, ale vyústilo to v to, že si paměť nechtěně přepisovali. To už mi snad přijde lepší, když tam bude únik paměti a program je nakonec sestřelen OOM killerem, než aby tam byla potenciálně bezpečnostní díra v podobě přetečení.

Samozřejmě ideál je to napsat tak, aby paměť neunikala, žralo jí to třeba o něco víc, ale konstantně, a bylo to bezpečné.

for ( i = 0; i < strlen(string); ++i ) {

Někdy se iteruje v opačném směru, k nule, což je trochu nezvyk, ale vlastně proč ne (pokud nezáleží na pořadí).

Nebo si klasicky uložit délku do proměnné.

o 4kB poli v lokální proměnné nemluvě

A jak by to mělo být správně? Alokovat dvojnásobek délky vstupu + uvozovky a nulový bajt? (jak navrhuje Kruci pod článkem) Nebo si alokovat nějakou dynamickou strukturu (jakou?) o velikosti tak 120 % vstupu, protože tam se to většinou vejde (a případně se ještě zvětší).

Do not double escape names. Fixes #36

Njn, chyba tam zůstala dál, jen se přestala projevovat.

[Kvakor]

for ( i = 0; i < strlen(string); ++i ) {

Někdy se iteruje v opačném směru, k nule, což je trochu nezvyk, ale vlastně proč ne (pokud nezáleží na pořadí).

Nebo si klasicky uložit délku do proměnné.

Se zapnutými optimalizacemi by měl překladač toereticky uložit výsledek strlen(string) do neviditelné automatické dočasné proměnné (popř. registru) a tu používat v cyklu, i když osobně bych na to raději nespoléhal - překladač musí mít jistotu, že vrácená hodnota je konstantní po celou dobu cyklu (tj. např. žádná pointerová aritmetika nebo předávání dalším funkcím, pokud není parametr const), v opačném případě bude volat strlen(string) při každém průchodu. Když už si někdo nechce zaplácávat zásobník, může proměnné použité k uchování délky řetězce nastavit jako register a doufat, že to překladač pochopí správně a nebude jí alokovat žádnou paměť.

A i když iterovat "pozadu" je sice z algoritimického hlediska ekvivaltení iterování směrem vpřed, procesor, cache a paměti apod. jsou dělané na to, že se paměť čte směrem dopředu, ne odzadu (viz prefetch),

[Josef Kufner]

Ta dynamická struktura je celkem snadno implementovatelná. Stačí si projít ukázkový příklad v dokumentaci vsprintf(), ta zajímavá funkce se jmenuje realloc(). Na začátku si alokuješ těch 120% a když to nebude stačit, tak jen string = realloc(string, 2 * strlen(string)) a hotovo.

[johniez]

A pro pripady, kdy by realloc selhal, je dobry si ulozit puvodni hodnotu pointru string jeste nekam jinam (jinak nebude mozny uvolnit puvodni pamet).

[Josef Kufner]

Nějak se mi tam nechtělo psát to "Pro názornost zjednodušeno."

[biolog]

Jazyk C (na rozdíl od C++) nic nezakrývá, takže programátor má neomezenou možnost napsat kód přesně, jak chce. Ale pak to vede na to, že programátoři vyrábí statická pole, protože C nemá prostředky (kromě komentářů) jak říct, že volající musí vrácený buffer s escapovaným stringem uvolnit. (V C++ byste vrátili std::string, ve vyšších jazycích ta chyba napsat ani nejde.) Ale hlavně máte moc! Cítíte ji?!

Asserce by taky byla na místě, jak navrhl Jindřich Makovička.

[Kkk]

Nevím proč, ale velmi dlouho jsem přemýšlel, jak se hledají chyby v autorovi softwaru.

[mankind_boost]

Tyhle věci se většinou debugují pomocí (f)MRI.

[Jindřich Makovička]

Kristova noho. U těch statických proměnných by mělo být asi tak milión varování a hlavně assert, že char *string náhodou nesměřuje někam do csv[]. Plus vstup i výstup const char *. A proč je statický i a ind, to mi už vůbec hlava nebere.

Celkem si dovedu představit, že to blbne, když se escapuje dvakrát a zdroj je tudíž stejný jako cíl.

[hulmiho-ukolen]

Krisztusza nohosz >8~D , iszten-be hazso,piscabo lofasz fuledbe A$ tuzedesz holocsko @&#°+

[xkucf03]

assert, že char *string náhodou nesměřuje někam do csv[]

Jenže pak by nešlo escapovat dvakrát. V tomto programu to asi není potřeba, ale jinak se to hodí (např. když chci někam vložit ukázku, jak vypadá escapovaný kód).

Pro mě má funkce prostě nějaký definiční obor a nějakou návratovou hodnotu a můžu ji zanořit třeba stokrát a pořád by měla fungovat. Proč by měla mít takové omezení jenom kvůli tomu, jaký jazyk jsem si pro implementaci vybral?

Dobré řešení asi je předávat ten buffer zvenku jako parametr a uvnitř nic nealokovat, ale pak to zase není funkce, ale spíš procedura.

[Mrkva]

Dobré řešení asi je předávat ten buffer zvenku jako parametr a uvnitř nic nealokovat, ale pak to zase není funkce, ale spíš procedura.

Hlavne nevis jak velky ho budes potrebovat.

[xkucf03]

To je pravda – musím znát implementační detaily té funkce (od kterých bych měl normálně abstrahovat), abych určil maximální velikost výstupu. Jak to tedy napsat správně?

[Mrkva]

A co je spatneho na tom ze vrati novy buffer nad kterym pak musim sam zavolat free()? Pripadne pokud uz jsem zhyckany tak knihovna muze drzet pointery na vsechny naalokovane buffery u sebe a uklidit po sobe pri zavolani nejake cleanup funkce.

[xkucf03]

Na tom mi nesedí, že se paměť uvolňuje někde jinde, než kde byla alokována, resp. že je za to odpovědný někdo jiný a je to hodně náchylné k chybám. Ale asi je to v C normální…

[Jindřich Makovička]

Jenže tenhle kód ani neumí escapovat dvakrát za sebou - jakmile narazí na uvozovky, tak je zapisuje na následující pozici a protože cíl==zdroj, escapuje stále dokola, dokud z bufferu nevyjede. Pokud potřebujete escapovat dvakrát, bylo by potřeba rozhraní ala snprintf, tj. (char *dest, int n, const char *src).

[Sid]

tato veta ma teda rozsekala "A ty statické lokální proměnné jsou taky divné – asi nějaká céčkařská optimalizace, které jako javista nemůžu rozumět…" .

[Jardik]

To psalo nejaky prase ne? Jak se vubec neco takoveho muze dostat do nejake distribuce??

[xkucf03]

BTW: dá se to nějak automaticky analyzovat nebo to musí všechno pročítat člověk? Odhalí nějaký nástroj tyhle problémy?

[johniez]

Klee?

[Jindřich Makovička]

Něco málo najde cppcheck, taky neškodí zkusit kromě GCC i Clang (oboje s -Wall případně -Wextra). Za chodu potom Address Sanitizer a Memory Sanitizer, viz každá zprávička o Google Chrome :)

[hz]

Tyhle selfpromo zpravicky (jeste v kategorii zajimavy clanek) - nevim co si o tom myslet

[xkucf03]

Taky bych radši, kdyby zprávičky psal někdo jiný ve třetí osobě

[Bystroushaak]

Hoď příště i do blogu a problém je vyřešen.