C vs C++ bei OSDEV


  • Mod

    MrX ist einer der führenden Developer von PrettyOS. Wenn er sagt, dass die Resultate wenig hilfreich sind, dann ist dem so.



  • Die Behauptung ist gewagt, ehenkes.

    Ja, ich weiss, dass man Tausende Warnungen bekommt, aber ich habe die Erfahrung gemacht, dass jede Warnung einen Bug bedeuten kann... d.h. in den 5000 Warnungen würde ich jetzt 5000 potentielle Bugs sehen...

    Ein Teil der Meldungen kam daher, das es nicht mit C99 oder GCC-spezifischen Dingen klarkam (leider brach er daraufhin jedes Mal ab - ich musste jede Datei separat checken). Ein Problem, das cppcheck übrigens nicht hat. Dann waren die meisten Fehlermeldungen so formuliert, das man nicht verstand, was es von einem will. Oft beklagte er sich auch nur, weil man selbst einige Dinge definiert, die ein Anwendungsprogramm einfach aus der Standardlib beziehen würde (malloc, free, size_t, ...). Richtig war allerdings, das man keine Namen mit _ am Anfang verwenden sollte, da das für den Compiler "reserviert" ist.



  • Ich habe mal splint über euren Quellcode laufen lassen, weiss jetzt nicht, welche Version, aufgerufen mit diesen Parametern:

    splint -I /tmp/prettyos/trunk/Source/user/user_tools -I /tmp/prettyos/trunk/Source/kernel -booltype bool +nolib +gnuextensions +trytorecover blabla.c
    

    und da sagt splint mehrmals "A memory leak has been detected" u. ä. oder Rückgabewerte von Funktionen unbenutzt, oder unsigned mit signed gemischt, oder untereinander nicht kompatible Datentypen gemischt u. ä. Hört sich alles nicht so gut an 🙂


  • Mod

    "A memory leak has been detected"

    <--- das ist interessant. Wir kontrollieren ja bereits mit dem heap logger und überwachen auch den physischen Speicher auf leaks. Da ist mir momentan nix aufgefallen.



  • ehenkes: Sämtliche Log-Funktionen zeigen nur dann was, wenn das Leak tatsächlich eintritt, und das ist oft nur in seltenen Fällen der Fall.

    abc.w:

    Rückgabewerte von Funktionen unbenutzt

    Halte ich für gängigen Stil und korrekt. (Nagut, wenns malloc wäre natürlich nicht 😉 )

    Das memory-leak ist da interessanter. Leider funktioniert Splint auch mit den diversen Parametern von dir und einem Update auf 3.1.2 (Vorher gabs für Windows nur uralt-Versionen) nicht, auch wenn sich die Lage durchaus gebessert hat. Er krepiert jetzt in ckernel.c Zeile 126, immerhin schafft er nun os.h und types.h

    Die Leaks hat er mir (als ich jede Datei einzeln bearbeitet habe) nicht gezeigt, was aber auch einfach daran liegen kann, das er nie so weit gekommen ist, weil er vorher an irgendwas verreckt ist.

    Vielleicht könntest Du die Ausgabe von Splint uns irgendwie zukommen lassen? (Vielleicht bereinigt um die Funktionsrückgabewert-Sache und andere nebensächliche Dinge). Wär gut, dann hätten wir eine Chance, die Splint-Ergebnisse zu verwerten 🙂


  • Mod

    Sämtliche Log-Funktionen zeigen nur dann was, wenn das Leak tatsächlich eintritt

    Das ist korrekt.

    Die Ergebnisse von splint bezüglich possible mem leaks würde ich gerne sehen.



  • Hier, bitte schön (wie gesagt, weiss nicht, welche PrettyOS Version, hatte alles im /tmp Verzeichnis entpackt):

    splint schrieb:

    /tmp/prettyos/trunk/Source/kernel/list.c: (in function list_Create)
    /tmp/prettyos/trunk/Source/kernel/list.c:17:20:
    Implicitly only storage hd->tail (type element_t 😉 not released before
    assignment: hd->tail = 0
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/scheduler.c: (in function scheduler_unblockEvent)
    /tmp/prettyos/trunk/Source/kernel/scheduler.c:52:5:
    Implicitly only storage blockedTasks->current (type element_t 😉 not
    released before assignment: blockedTasks->current = blockedTasks->begin
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/storage/ehci.c:60:2:
    Fresh storage bar not released before return
    A memory leak has been detected. Storage allocated locally is not released
    before the last reference to it is lost. (Use -mustfreefresh to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/storage/ehci.c:44:71:
    Fresh storage bar created
    /tmp/prettyos/trunk/Source/kernel/storage/flpydsk.c: (in function createFloppy)
    /tmp/prettyos/trunk/Source/kernel/storage/flpydsk.c:140:5:
    Implicitly only storage fdd->drive.type (type portType_t 😉 not released
    before assignment: fdd->drive.type = &FDD
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/synchronisation.c: (in function semaphore_create)
    /tmp/prettyos/trunk/Source/kernel/synchronisation.c:17:5:
    Implicitly only storage obj->resources (type task_t **) not released before
    assignment: obj->resources = malloc(sizeof(task_t 😉 * obj->resCount, 0,
    "semaphor-resources")
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/paging.c:44:41:
    Owned storage kernel_pd->pd_phys_addr passed as <error> param:
    kdebug (..., kernel_pd->pd_phys_addr)
    The owned reference to this storage is transferred to another reference
    (e.g., by returning it) that does not have the owned annotation. This may
    lead to a memory leak, since the new reference is not necessarily released.
    (Use -ownedtrans to inhibit warning)
    /tmp/prettyos/trunk/Source/kernel/cdi/cdi_storage.c: (in function cdi_storage_device_init)
    /tmp/prettyos/trunk/Source/kernel/cdi/cdi_storage.c:52:2:
    Fresh storage partitions not released before return
    A memory leak has been detected. Storage allocated locally is not released
    before the last reference to it is lost. (Use -mustfreefresh to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/cdi/cdi_lists.c: (in function cdi_list_pop)
    /tmp/prettyos/trunk/Source/kernel/cdi/cdi_lists.c:32:20:
    Last reference temp to implicitly only storage list->head not released
    before return
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/cdi/cdi_lists.c:30:5:
    Original reference lost
    /tmp/prettyos/trunk/Source/kernel/cdi/cdi_lists.c:44:24:
    Fresh storage temp not released before return
    A memory leak has been detected. Storage allocated locally is not released
    before the last reference to it is lost. (Use -mustfreefresh to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/cdi/cdi.c:34:22:
    Fresh storage driver (type struct cdi_driver 😉 not released before
    assignment (in post loop test): driver = cdi_list_get(drivers, j)
    A memory leak has been detected. Storage allocated locally is not released
    before the last reference to it is lost. (Use -mustfreefresh to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/cdi/cdi.c:34:22:
    Fresh storage driver created
    /tmp/prettyos/trunk/Source/kernel/task.c: (in function tasking_install)
    /tmp/prettyos/trunk/Source/kernel/task.c:43:5:
    Implicitly only storage kernelTask.page_directory (type page_directory_t 😉
    not released before assignment: kernelTask.page_directory = kernel_pd
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/keyboard.c:40:5:
    Implicitly only storage KQ->pHead (type uint8_t 😉 not released before
    assignment: KQ->pHead = KQ->buffer
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/filesystem/fsmanager.c: (in function formatPartition)
    /tmp/prettyos/trunk/Source/kernel/filesystem/fsmanager.c:31:2:
    Fresh storage part not released before return
    A memory leak has been detected. Storage allocated locally is not released
    before the last reference to it is lost. (Use -mustfreefresh to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/filesystem/fsmanager.c:29:44:
    Fresh storage part created
    /tmp/prettyos/trunk/Source/kernel/filesystem/fsmanager.c: (in function fopen)
    /tmp/prettyos/trunk/Source/kernel/filesystem/fsmanager.c:43:5:
    Implicitly only storage file->volume (type partition_t 😉 not released
    before assignment: file->volume = getPartition(path)
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/filesystem/initrd.c: (in function ramdisk_install)
    /tmp/prettyos/trunk/Source/kernel/filesystem/initrd.c:31:5:
    Implicitly only storage RAMdisk.type (type diskType_t 😉 not released
    before assignment: RAMdisk.type = &RAMDISK
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/video/vbe.c:53:17:
    New fresh storage (type task_t 😉 passed as implicitly temp (not released):
    create_vm86_task(((void *)0x121))
    A memory leak has been detected. Storage allocated locally is not released
    before the last reference to it is lost. (Use -mustfreefresh to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/video/console.c:34:5:
    Implicitly only storage kernelConsole.sp (type struct semaphore 😉 not
    released before assignment: kernelConsole.sp = semaphore_create(1)
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/kheap.c: (in function heap_install)
    /tmp/prettyos/trunk/Source/kernel/kheap.c:50:5:
    Only storage assigned to unqualified static:
    regions = malloc(0, 0, "heap-start")
    The only reference to this storage is transferred to another reference (e.g.,
    by returning it) that does not have the only annotation. This may lead to a
    memory leak, since the new reference is not necessarily released. (Use
    -onlytrans to inhibit warning)
    /tmp/prettyos/trunk/Source/kernel/todo_list.c: (in function todoList_create)
    /tmp/prettyos/trunk/Source/kernel/todo_list.c:12:5:
    Implicitly only storage list->queue (type listHead_t 😉 not released before
    assignment: list->queue = list_Create()
    A memory leak has been detected. Only-qualified storage is not released
    before the last reference to it is lost. (Use -mustfreeonly to inhibit
    warning)
    /tmp/prettyos/trunk/Source/kernel/syscall.c: (in function syscall_handler)
    /tmp/prettyos/trunk/Source/kernel/syscall.c:136:5:
    Only storage currentTask->console assigned to unqualified:
    currentConsole = currentTask->console
    The only reference to this storage is transferred to another reference (e.g.,
    by returning it) that does not have the only annotation. This may lead to a
    memory leak, since the new reference is not necessarily released. (Use
    -onlytrans to inhibit warning)

    Bei mir bricht splint übrigens auch wegen "syntax errors" ab. Ich habe damals auf die Schnelle ein Skript zusammengefrickelt, ungefähr so:

    find . -name *.c -type f > tmp.sh
    

    und dann mit dem Editor meiner Wahl splint-Kommandos eingefügt, das geht mit dem Editor meiner Wahl in Sekunden 😉



  • Und? Hat splint recht oder ist das alles Quatsch?



  • Erhard und ich haben uns das gestern Abend/heute Nacht angesehen und haben keinen Fehler finden können.

    Splint sieht in Pointern anscheinend nur einen Sinn: dynamische Speicherallokation. Insofern geht es davon aus, das jede Funktion, die einen Pointer zurückgibt Speicher allokiert hat, was dazu führt, das diese ganzen Meldungen zurückgegeben werden.
    Ausserdem scheint es der Meinung zu sein, das man die Member frisch allokierten Speichers erstmal freigeben soll, bevor man sie initialisiert. (Das ist beispielsweise der "Fehler" in list.c Zeile 17)

    Trotzdem natürlich danke für den Log, zu überprüfen ob es vielleicht doch Fehler sind hat ja nicht geschadet. 🙂



  • Mr X schrieb:

    ... zu überprüfen ob es vielleicht doch Fehler sind hat ja nicht geschadet. 🙂

    Nichts zu danken, dafür ist ja dieses Tool da, warnt vor allen möglichen Sachen, ist zwar dumm wie Stroh, hilft aber manchmal 🙂


Anmelden zum Antworten