Indrømmelse: Jeg er hysterisk med C/C++ kode

Jeg har hen over de sidste par måneder været med i et projekt, hvor vi skulle rydde op i noget C/C++ kode på Linux, hvor der var en del problemer. I dette blog-indlæg sætter jeg fokus på hvor "hysterisk" man kan sætte sin GCC/GLANG C eller C++ compiler op, og der er ikke fokus på dynamisk kode-check. Det regner jeg med at vende tilbage til i et andet blogindlæg.

Basic-flag

Jeg startede projektet med at tilføje compile flag til GCC så vi fik fockus på de værste problemer i koden. Jeg starter altid med "-Wall", som giver en hel del advarsler af forskellig art, men f.eks. får man ikke advarsler om variable som skygger for hinanden eller manglende prototyper i headerfiler. Derfor starter jeg også med "-Wshadow -Wimplicit-function-declaration"

CFLAGS := -Wall -Wshadow -Wimplicit-function-declaration 

Strammer op

Efter at der kom styr på at alle basis-fejl blev rettet op, gik jeg mere hårdt til værks, med at lede efter ubrugte variable, eller steder hvor resultater ikke blev brugt, type-konverteringsfejl og endelig tre hystade-flag "-ansi -pedantic -Wextra".

-Wunused-variable -Wunused -Wunused-result -Wconversion -ansi -pedantic -Wextra

Det skal lige nævnes at "-pedantic" er koblet til "-ansi", og måske er det mange sammenhænge smartere at se på f.eks. "-std=c99" eller en tilsvarende C++ standard, da "-ansi" bl.a. blokerer for v

For at få fokus på at selv advarsler blev undersøgt, konverterer jeg advarsler til at blive flaget som fejl.

-fmax-errors=1 -Werror 

Hvad betød det at være så hysterisk?

Det har været hårdt at rette koden op, så der ikke er advarsler tilbage, men det var også en rigtig god oplevelse at rette koden op. Langt det meste, der kom op i søgelyset var fejl ala.

int c = 1;
uint d = c; 

Det er kode som næsten altid er ok, men var "c" initialiseret med et negativt tal, så kan det være mere "underholdende".

Der var også få tilfælde, hvor mere subtile men rigtige fejl pludselig kom til overfladen.

Efter at være kommet i bund med fejl og advarsler har det været skønt at arbejde med koden fordi nye advarsler drukner i listen af gamle advarsler. Nu ved at jeg en advarsel er noget nyt og noget at jeg bør kigge på. Overraskende har der ikke været surhed fra de andre udviklere over at køre så hysterisk. Alle har taget godt i mod det, men det var en hård fase at luge alle advarsler ud.

Et godt trick til at skrive makefiler er også at lave understøttelse for forskellige niveauer af advarsler, så man i en stor restruktureringsøvelse kan køre med lavt niveau af advarsler og senere i et udviklingsforløb få de mere detaljerede advarsler frem.

Andre GCC/G++ flag

Der er tonsvis af flag man kan give GCC og har du haft god anvendelse af andre compilerflag, så skriv gerne om dem nedenfor.

En anden sjov erfaring er at selvom jeg f.eks. har slagtet alle advarsler ned for en compiler, så er min erfaring at forskellige compilere giver forskellige advarsler. De nyeste udgaver af Visual Studio giver nogle valide advarsler, som GCC ikke fanger. At hoppe mellem forskellige compilere er guld.

/pto

Relateret indhold

Kommentarer (24)
Kenneth Geisshirt

Jeg oversætter også gerne med både clang og gcc. Jenkins er din ven - oversæt altid med begge compilers hver gang og opfat warnings som fejl.

Men som en sjov øvelse, er det også værd at oversætte på forskellige platforme, f.eks. Intel og ARM (en billig Raspberry Pi 2) og køre dine tests. Du kan fange fejl grundet i subtile antagelser og typer og værdier af variable.

Morten Jensen

De eneste warnings der tillades i den C-kodebase jeg udvikler professionelt, er dem der indsættes med #warning-direktivet. FWIW: vi skriver embedded functional safety critical software.

Udviklerne overser ofte warnings om reelle fejl, som f.eks. 'a=b' i stedet for 'a==b' hvis der er nok warnings til at man skal scrolle for at se dem allesammen... Derfor har vi nul warnings som mål. Ellers er erfaringen, at det stikker af når projektet begynder at nærme sig en vis størrelse, fx. 100.000 liniers kode.

Og hvad angår dynamisk checks, så strør jeg om mig med assertions! De kan i mange tilfælde verificeres statisk i embedded kode, f.eks. med CBMC.

Morten Siebuhr

Jeg har haft ganske gode erfaringer med at tilføje ca. ditto linting til en håndfuld projekter efterhånden.

Siden risikoen ved et mega-commit med alle rettelser ofte er for stor, kan man sætte linter/compiler/hvadnu til at ignorere alle advarsler/fejl, men derved få CI/precommit/... infrastrukturen på plads. Husk endelig at have den tilhørende konfiguration i versionskontrol - så er der bred enighed, kan tweakes per-projekt og nemt opdateres over tid.

Derefter kan man slå et flag til ad gangen og i samme commit rette alle opståede fejl. Så får man ét commit med det ændrede flag og alle deraf følgende rettelser. Og der en god anti-overspringshandling at tilføje endnu et flag en gang imellem ☺.

Casper Bang

Har du prøvet med Intel's propriotære compiler? Ud over optimering performance er de også kendte for at have gode warnings. Kræver selvf. at du er på x86/amd64 hvilket historien ikke melder noget om.

Ivan Skytte Jørgensen

HP har et værktøj "cadvise" (code advisor). På pa-risc var det et standalone værktøj, men på itanium er det bygget ind i compileren. Det interessante er at compileren på pa-risc var deres eget produkt, men på itanium var den baseret på Intel-teknologi og havde ret meget til fælles med Intels compiler. Så mit spørgsmål er om Intel i deres toolset har "cadvise" eller om det var en ren HP-tilpasning?
Det interessante ved cadvise, som jeg aldrig nåede at prøve da jeg havde adgang til det, er at man angiveligt kan skrive plugins til det som har adgang til hele parse-træet, inkl på link-tidspunkt, så man kunne lave nogle ret avancerede globale checks.

Ivan Skytte Jørgensen

Sidst jeg prøvede (et par år siden), da havde Intels compilere bedre warnings end gcc. Så vidt jeg ved er den oprindeligt baseret på Kai C/C++ compiler, som de købte tilbage i 2000. Studerende, undervisere og opensource-udviklere kan få en gratis version. Andre må betale $699, hvilket stadig er billigt hvis den finder blot en fejl i ens program inden en kunde gør det.

På mit tidligere arbejde fik mine C++-programmer en tur med GCC 2.95.3 og 4.x, HP aCC 02.xx og 06.xx, clang, Flexelint, samt Insure++ (når jeg havde tålmodighed til det).

Men hvis man er seriøs med at finde fejl i kode vha. statisk analyse, så bør man investere i f.eks. Flexelint, som er beregnet til at gøre programmører kede af det :-) Se evt. min præsentation fra debugging-konferenced i 2013: http://i1.dk/misc/flexelint/

Troels Liebe Bentsen

Nu har jeg ikke så meget erfaring med C udvikling, men jeg har brugt Coverity Scan til mit open source Java project og der har den fundet et 2-3 rigtig fejl og 1-2 conercases, plus lidt import sjusk. Så de par timer det to at få sat op med integration til build pipeline var godt givet ud. Og da Coverity er kendt for statisk analyse af C kode så burde der jo også være noget at hente der?

Eksemple med Travis-CI og Coverity scan kan ses her:
https://github.com/tlbdk/copybook4java

Thomas Søndergaard

Peter, du er ikke spor hysterisk. Her er vores C++ compiler flag:

Linux gcc:
-std=c++11 -march=core2 -fno-common -pipe -Werror -Wall -Wextra -Wreorder -Wnon-virtual-dtor -Wpacked -Wcast-align -Wenum-compare -Wpointer-arith -Wunused -Wuninitialized -Winit-self -Winvalid-pch -Woverlength-strings -Woverloaded-virtual -Woverflow -Wno-unknown-pragmas -Wpacked-bitfield-compat -Wlogical-op -Wsync-nand -Wstrict-null-sentinel -Wnoexcept

Visual Studio 2013/2015:
/Zc:inline /Zc:rvalueCast /volatile:iso /Zo /MP /WX /arch:SSE2 /nologo /Zm200 /W4

Derudover bygger vi også med clang og vi har cppcheck og clang-tidy på vores CI system til yderligere statiske checks. Vi har også længe snakket om at prøve Coverity og/eller PVS-studio.

Vi har brugt -Wall -Werror i mange år, mens yderligere statisk analyse værktøjer kom på bordet efter at have læst om Carmacks erfaringer i 2011:

http://www.gamasutra.com/view/news/128836/InDepth_Static_Code_Analysis.php

Mogens Hansen

Så vidt jeg ved er den oprindeligt baseret på Kai C/C++ compiler, som de købte tilbage i 2000.

Intels C++ compilere bruger EDG (Edison Design Group - www.edg.com - https://en.wikipedia.org/wiki/Edison_Design_Group) som front-end.
Den bliver brugt i en række sammenhænge - bl.a. til code completion i Microsoft Visual C++ IDE'et.

EDG havde i mange år den mest komplette implementering af C++.
De var bl.a. de første (og eneste ?) som implementerede "export" - på trods af de havde været imod indførelsen af det i standarden. Det er god stil.

Mogens Hansen

Det lyder ikke hysteriskt - det lyder mere som almindelig, håndværksmæssig forsvarlig opførsel.
Det burde ikke give anledning til surhed fra kollegaer at have sådan en politik og hjælpe projektet til at komme i en tilstand, hvor det er praktisk muligt. Jeg mener det er et organisatorisk problem hvis det ikke var sådan.

Rasmus Andersen

Godt indlæg!
Jeg har også presset "hysteriet" ned over vores software-afdeling. Her var der dog en stor gammel kodebase, som vi i første omgang lader køre videre og så fremadrettet kræver at alt compilerer rent. Hertil er "GCC Diagnostic pragma" (https://gcc.gnu.org/onlinedocs/gcc/Diagnostic-Pragmas.html) nyttigt så man selektivt kan enable/disable diagnostik på diverse filer.

Baldur Norddahl

Jeg er lidt overrasket over at der ikke er nogen af læserne som kryber til korset og indrømmer; jeg har koden, som slet ikke er clean og bliver det heller aldrig...

Jeg er skiftet til Rust...

Men ellers har vi vel alle været nødt til at arbejde på kodebaser som ikke er vores egne og hvor det vil være uoverkommeligt at rydde op i alt. Specielt hvis man bare skal lave en mindre rettelse.

Allan Jensen

I de flest større projekter jeg har arbejdet på har vi haft en politik om at oversætte med WallWerror, men det skal siges at det er en del arbejde. Hver gang der kommer en ny compiler version er der nye warnings der skal håndteres eller white-listes. Og der skal ofte white-listes en hel del warnings i moderne compilere. Den jeg rammer oftest nu om dage med GCC, er en rigtigt smart logik der regner ud om en 'int' på et givent tidspunkt kun kan være positiv og så giver en "hjælpsom" warning hvis man laver et mindre-end-nul check, desværre får man den også på mindre-end-nul checks i inline functions som kan kaldes fra flere steder og generelt giver mening, også selvom compileren nu er så klog at den kan regne ud når den ikke gør.

Nikolaj Brinch Jørgensen

At man skal tilføje så meget ekstra til sin compiler, for at fange fejl og dårligdomme. Dvs. default er at man slipper afsted med dårligt håndværk/sjusk.

Nå et spørgsmål til Peter. I fik gjort rent, men fik I også rydtet op? Oftests synes jeg at når kode lider af enormt meget sjusk og dårligt håndværk, så lider det generelle design og kodens struktur i al almindelighed enormt, og der skal virkelig tages fat (eller måske nogen gange smides ud).

Hvad med automatiserede tests, var der nogen af dem, og kunne de bistå jer i arbejdet? Var i nødt til at skrive nogen, inden i gik i gang for at kunne verificere jeres rengøring/oprydning ikke havde ødelagt noget?

Log ind eller Opret konto for at kommentere