[so] Greseli la tema 1

Thu Feb 8 20:21:37 EET 2007

        Scriu acest mail pentru că m-am apucat să corectez temele 1 (şi sper să le şi termin la timp :-D, ţinând cont că sunt şi eu în sesiune) şi am găsit unele greşeli extrem de frecvent. Să nu credeţi că dacă tema vi s-a părut uşoară şi aţi luat note mari este ok. În multe cazuri s-au luat note mari pentru că testele se pare că erau prea uşoare şi pentru că m-am ţinut după barem. În multe cazuri notele nu reflectă calitatea temelor. Mailul ăsta ar trebui să vă facă să vă simţiţi puţin stânjeniţi dacă sunteţi cu musca pe căciulă şi, în cel mai bun caz, să aflaţi ce puteţi face mai bine în viitor (de exemplu când veţi fi anganjaţi unii dintre voi). Dacă vreţi doar nota la această materie puteţi foarte bine să nu citiţi ce urmează. Cei care nu sunt vizaţi de acest mail o să se recunoască singuri.

        De departe cea mai frecventă greşeală este buffer overflow. Vă mărturisesc sincer că până acuma nu înţelegeam cum de poate fi o problemă reală, dar când am văzut cât de mulţi sunt comozi când scriu cod şi nu scriu 2-3 linii în plus doar ca să termine mai repede, m-am lămurit cum stă treaba şi de ce aşa de multe programe cunoscute au bug-uri. Dacă nu ştiţi ce înseamnă buffer overflow şi de ce e rău (mai ales când este exploatabil, dar nu numai), puteţi căuta pe net (există şi o carte destul de bună, "Writing Secure Code" cu ceva detalii şi exemple). Oricum, chiar dacă nu ştiaţi, folosirea funcţiilor strcpy şi strcat tot greşită e (pentru că ele nu verifică depăşirile (nu au cum), trebuie să verificaţi voi pentru ele). Vă rog să nu mai folosiţi buffer-e (liste de parametri, etc.) de dimensiune fixă unde nu e cazul şi să nu mai folosiţi funcţiile din string.h până nu aflaţi ce e aia un buffer overflow şi cum se scrie codul corect. Dacă o să ajung să folosesc software scris de voi, vă mai rog încă o dată. :-P

        Pe locul doi sunt memory leak-urile. malloc fără free înseamnă că risipiţi resursele sistemului inutil, şi, în cel mai rău caz, expuneţi aplicaţia la DoS. Unii au evitat leak-uri fără să îşi dea seama, pentru că au făcut malloc chiar înainte de exec (pun pariu că mulţi nu ar fi dat free, chiar dacă lipsea exec-ul de acolo). Mulţi au uitat că au malloc în funcţiile care expandează variabile de mediu şi nu au mai dat free decât în unele cazuri. Dacă vi se pare prea greu managementul memoriei alocate dinamic (nu era cazul în 90% din temele 1 pe care le-am văzut), treceţi pe C++ şi nu folosiţi pointeri, ci doar copieri de obiecte, sau folosiţi liste de obiecte alocate dinamic. Se găsesc soluţii. Cel mai uşor e să citiţi odată codul şi la fiecare malloc să vă întrebaţi unde daţi free. Apoi să verificaţi cu unelte automate că aţi avut dreptate.

        O altă greşeală, la care nu mă aşteptam deloc, şi care poate e şi din vina noastră că nu v-am explicat la laborator, sau a voastră că nu aţi citit man şi nu aţi fost la curs :-P, este că aţi specificat foarte mulţi drepturile de acces la funcţia open, chiar dacă lipsea O_CREAT. Dacă vă gândiţi puţin şi aţi înţeles ce e aia drept de acces la fişiere vă daţi seama care e problema. Desigur, asta poate fi din cauză că aţi dat copy/paste la cod (am văzut şi O_CREAT la input: discutabil), ceea ce ne duce la altă problemă pe care am observat-o: unii dintre voi au incredibil de mult cod duplicat. E preferabil să faceţi funcţii în acest caz. Cred ca unele teme s-ar fi redus la 75% cu 1-2 funcţii definite în plus.

        Altă problemă este că mulţi nu au înţeles ce face dup2. dup2 închide vechiul file descriptor, deci dacă acolo era deschis stdin, de exemplu, un close ulterior nu o să-l deschidă la loc. Trebuie fie să-l salvaţi în alt file descriptor, fie să faceţi dup2 după fork şi înainte de exec. Multe implementări ar fi eşuat pe un test care făcea o redirectare într-un fişier şi pe urmă rula încă o comandă fără acea redirectare. Vă rog să citiţi manualul funcţiilor pe care le folosiţi cu atenţie.

        Ceea ce ne duce la următoarea problemă: mulţi nu au citit cu atenţie comentariile din parser.h, şi fie nu au folosit câmpul expand (şi au expandat întotdeauna variabilele de mediu), fie au neglijat că şi câmpul verb putea fi alcătuit din mai multe părţi. Doar dacă în teste nu apare aşa ceva nu înseamnă că nu poate ieşi din parser aşa ceva (culmea e că se poate să iasă şi chiar e de dorit :-P). Oameni buni: orice chestiuţă (cât de mică o fi ea) pe lumea asta, dacă vine cu documentaţie, asta înseamnă ceva. De obicei înseamnă că e bine să citiţi cu atenţie documentaţia (complet). Chiar şi la un card de memorie, un ceas, un set de baterii, etc., darămite la o funcţie despre care nu ştii ce face decât dacă citeşti documentaţia (repet: complet şi cu atenţie).

        Mai e un set de probleme care ţin în particular de tema 1 sau sunt mai "exotice".

        Cea mai mare problemă ar fi faptul că mulţi care au rezolvat tema pe linux au folosit "orbeşte" fork recursiv, doar pentru că aşa era parcurs arborele în exemplele parser-ului, fără să pună în balanţă avantajele şi dezavantajele. Cel mai mare dezavantaj este că se face mai greu comunicarea cu noua instanţa a shell-ului. Asta duce la probleme: exit care nu face shell-ul să iasă, chdir care nu are efect, etc. cu soluţii incomode (pipe-uri de comunicare între procese) sau greşite (exit code particular pentru a comunica cu părintele, exit code ce ar putea fi la fel de bine generat de un program). Nu spun că nu se poate face ceva corect aşa, doar ca prea mulţi au făcut fără să se gândească la alternative şi mulţi dintre ei au multe scăpări la ce a rezultat.

        La fel cu "copierea" modelului de pe Linux pe Windows. Doar pentru că în linux *trebuie* să faci redirectările suprascriind in/out standard şi făcând apoi exec, nu înseamnă că aşa e cel mai bine pe Windows. Există un parametru special la CreateProcess pentru aceste redirectări. Asta evită apeluri inutile de sistem şi posibile erori de implementare. Care erori există. De exemplu (şi asta se aplică şi pe Linux) foarte puţini au luat în considerare un aspect ceva mai subtil de securitate: s-ar putea ca noul proces creat să moştenească handle-uri de care nu are nevoie şi la care nu ar trebui să aibă acces, doar pentru că un proces de dinainte trebuia să moştenească acel handle care a rămas deschis (valabil şi pe Linux pentru file descriptori). Asta e valabil şi pentru implementările care au folosit thread-uri şi acesta este motivul prinicipal pentru care eu nu aş recomanda thread-uri pentru această temă (pe Windows, că pe Linux mai sunt câteva, cum ar fi interacţiunea foaaaaarte ambiguă/slab documentată între fork şi lpthread).

        Alte greşeli au variat de la a încerca să faci tema să meargă doar pentru teste (nu pentru cazul general), până la implementarea incorectă a rulării în paralel (doar ca să iasă tema), wait doar după unii dintre copii (pentru că altfel era greu de făcut paralelismul), înţelegerea greşită a versiunilor funcţiilor ANSI/Unicode pe Windows, etc.

        În cazul în care doriţi să programaţi pe bani şi/sau ţineţi la calitatea lucrurilor pe care le faceţi vă rog să citiţi cu atenţie ce am scris şi să evitaţi în viitor greşelile pe care le-aţi făcut.

        Deoarece voi presupune că marea majoritate citiţi acest mail, nu o să mai fiu la fel de explicit în comentariile pe care le fac la teme de acum înainte (pentru cazurile frecvente pe care le-am spus mai sus). De exemplu, o să scriu doar "-0.1 leak", şi o să dau explicaţii doar acolo unde e un leak mai subtil. Eu nu o să fiu în stare să vă găsesc toate bug-urile, de obicei citesc temele doar o dată, aşa că dacă nu am găsit o problemă într-o temă nu înseamnă că nu există.


                Maximilian Machedon

PS: Ştiu că este "doar o temă", dar vă asigur că există o diferenţă între a scrie cod bun şi a scrie cod care să meargă. Adică dacă aţi scris codul doar că să meargă asta nu v-a ajutat cu nimic pentru a învăţa să scrieţi cod bun.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://cursuri.cs.pub.ro/pipermail/so/attachments/20070208/221c9b2a/attachment.html