Klassenfrage :)



  • @Schlangenmensch
    Hey danke für deine umfangreichen und detaiillierten Tips, mit denen hab ich nun auch Arbeit 🙂



  • @EL-europ sagte in Klassenfrage 🙂:

    Die erste compiler meldung bekomm ich auch (ohne die sanitizer) und bedeuted nur das er die Größe/Breite des Integerwertes, der zur Adresse addiert wird zur compileZeit nicht kennt. Das sit kein Problem meines erachtens in c, in c++ ist es vielleicht ein prop?

    Nein, es ist unklar, was void* für eine Breite hat. Wurde dir oben schon erklärt. Siehe auch https://stackoverflow.com/questions/3523145/pointer-arithmetic-for-void-pointer-in-c

    Die zweite meldung interpretiere ich so, das er den Rest des Strings einfach abschneidet?

    Wieder falsch. Es wird nicht magisch passend zurechtgekürzt. Es wird Speicher überschrieben, der dir (bzw. der Variable) nicht gehört. Undefiniertes Verhalten, alles kann passieren. Das Programm darf abstürzen, falsche Dinge tun etc.



  • @EL-europ sagte in Klassenfrage 🙂:

    Jawoll 😃 ich bekomm mit deinen flags nen haufen compilermeldungen mit denen ich mich nun auseinander setzten kann. Das -fsanitaze:memory funktioniert bei mir nicht, aber die Anderen werfent einen haufen Meldungen aus. Die versuch ich erst mal zu verstehen. Danke

    Ich schrieb, dass -fsanitize=memory nicht von GCC unterstützt wird, nur von Clang. Bei Bedarf einfach mit clang++ statt mit g++ kompilieren. Die sind weitgehend kompatibel und arbeiten bei allen grundlegenden Sachen mit den selben Parametern. Bestimmte Sanitizer kann man aber nicht zusammen verwenden, wie z.B. -fsanitize=address und -fsanitize=memory. Da müsste man das Programm mehrmals durchtesten, einmal mit dem einen und einmal mit dem anderen Sanitizer.

    Verstehe ich das richtig, das -fsanitize mir Meldungen(Abstürze) erst zur laufzeit "verursacht" ?

    Ja, wie @wob schon erwähnt hat. Oft hängen die Bedingungen, die zu einem Fehler führen, von erst zur Laufzeit bekannten Inputs ab und können nicht sinnvoll während des Kompilierens analysiert werden. Dafür sind die Sanitizer da. Für statische Analyse (Probleme nur anhand des Codes erkennen) gibt es die von @wob erwähnten Tools wie clang-tidy, cppcheck und andere. Es ist nicht verkehrt, die ebenfalls zu verwenden.

    Auch den Compiler aufzufordern sehr strikte Warnungen auszugeben und jede Warnung als Fehler zu interpretieren -Wall -Wpedantic -Werror (und eventuell sogar noch -Wextra) kann hilfreich sein, da es einen zwingt, sich zumindest mal mit potentiellen Problemen auseinanderzusetzen. Selbst wenn die Warnungen manchmal unproblematisch sind, wenn man weiss dass diese in ihrem jeweiligen Kontext nicht zu einem Problem führen können (da sollte man sich aber sicher sein).

    Ah blöde Frage, eben seh ich das er es gar nicht kompiliert mit diesem Flag

    Die Sanitizer werden aber schon untersützt oder? Ich glaube ein aktuelles Debian z.B. hat GCC 12, damit sollte das gehen. Die Version kannst du mit g++ -dumpversion herausfinden.

    Ansonsten sehe ich es auch so, dass es wohl besser ist das Progamm weitgehend neu zu schreiben anstatt "an den Symptomen he­r­um­zudok­tern". Die Sanitizer und die statischen Analysetools sollten dir allerdings helfen, dich auch davon zu überzeugen - oder dich zwingen, das implizit zu tun, wenn du das alles "fixen" willst 🙂



  • @Finnegan
    ja ich hab den g++ 12.2.0, der kennt auch kein #include <format>
    Mit Codeanalyse hab ich mich in solch gefordertem Ausmaß noch nicht beschäftigt, wird wohl Zeit?



  • @wob
    Es ist also Zufall das die thread-Funktion den richtigen x-wert bekommt?
    Und wie kann man es umsetzen so das es gutes c++ ist?



  • @EL-europ sagte in Klassenfrage 🙂:

    @Finnegan
    ja ich hab den g++ 12.2.0, der kennt auch kein #include <format>

    Features der Standardbibliothek werden mitunter abhängig vom C++-Standard aktiviert, mit dem der Compiler arbeitet. Ich weiss gerade nicht, was der Default-Standard für GCC 12 ist (könnte C++14 sein), aber std::format und den zugehörigen Header gibt es erst ab C++20. Du solltest diesen Explizit mit dem Compiler-Flag -std=c++20 aktivieren. Dann steht dir auch der <format>-Header zur Verfügung.

    Edit: Blödsinn, "Text formatting" und damit std::format scheint es laut dieser Support-Matrix tatsächlich erst ab GCC 13 und Clang 17 zu geben. Bei diesen gilt dann aber, was ich oben geschrieben habe. Das ist nur aktiv, wenn der Compiler in dem entsprechenden Sprachstandard arbeitet.

    Mit Codeanalyse hab ich mich in solch gefordertem Ausmaß noch nicht beschäftigt, wird wohl Zeit?

    Schadet gewiss nicht. Du scheinst da ja auch ein gutes Testprogramm zu haben, mit dem du sehr viel über das Thema lernen kannst 😛



  • @EL-europ sagte in Klassenfrage 🙂:

    ja ich hab den g++ 12.2.0, der kennt auch kein #include <format>

    std::format ist da noch nicht drin. Alternativ zu einem Kompiler-update könntest du fmtlib nehmen.
    Diese Lib kannst du dann bei Zeiten mehr oder weniger 1:1 gegen std::format austauschen (hoffe ich zumindest - so ist es bei uns geplant).



  • @Schlangenmensch
    Betreff:
    _Display *Display
    _Apple * Apple
    ... meint
    Ich soll im Konstruktor der _Userinterface ein eigenes Objekt "_Display" mit new instazieren? Das hab ich eben schnell versucht und zum Beginn der Laufzeit speicherzugriffsfehler erhalten. Oder meinst du eine andere Herangehensweise?



  • @Jockelx
    Der zweite zweck der Software ist mit dem Standart der Linuxdistro auszukommen, alle (auch ich) verschieben nur Registerinhalte und machen nix Neues nur anders. Ich will nicht von Fremdbibliotheken abhängen; Natürlich kann ich ohne Andere auch Nichts, aber es soll nicht an Softwarebibliotheken hängen.



  • @EL-europ Ich bin mir nicht sicher, was du meinst. Ich bin mir nur sicher, dass ich kein new gemeint habe.
    Wenn ein Userinterface ein Display haben soll, warum soll das denn auf dem Heap Alloziert werden und ein Pointer in Display sein?



  • @Schlangenmensch
    Achso* 🙂
    Du meinst es brauchen gar keine Objekte instanziert werden wenn ich sowieso nur auf einem "operiere"?

    Ich probier mal ohne "new" auszukommen, vermute aber das ich damit alles, auch die Aufrufe, ändern muss



  • @EL-europ sagte in Klassenfrage 🙂:

    Du meinst es brauchen gar keine Objekte instanziert werden wenn ich sowieso nur auf einem "operiere"?

    Ich denke @Schlangenmensch meint vermutlich so etwas hier:

    int main()
    {
        _Display Display;
        _Userinterface Ui{ BORDER, BORDER, Display, ui_callback };
        ...
    }
    

    das bedeutet nicht, dass keine Objekte instanziert werden, sondern dass sie als automatische Variablen hierbei direkt auf dem Stack liegen und nicht im Heap.

    Das bietet sich gerade bei denen besonders an, weil die Objekte zu Beginn des Programms einmal erzeugt werden und über die gesamte Laufzeit erhalten bleiben. Diese Objekte werden dann mit der schließenden geschweiften Klammer am Ende von main automatisch zerstört und deren Destruktoren aufgerufen. Kein delete notwendig.

    Der zweite zweck der Software ist mit dem Standart der Linuxdistro auszukommen, alle (auch ich) verschieben nur Registerinhalte und machen nix Neues nur anders. Ich will nicht von Fremdbibliotheken abhängen; Natürlich kann ich ohne Andere auch Nichts, aber es soll nicht an Softwarebibliotheken hängen.

    libfmt hat scheinbar auch einen "header-only"-Modus:

    • Optional header-only configuration enabled with the FMT_HEADER_ONLY macro

    Damit wäre das Einbinden sehr simpel, da nur das #include benötigt wird, ohne die Bibliothek extra bauen und linken zu müssen. Du könntest deren Quellcode z.b. in ein Unterverzeichnis deines Projekts kopieren, und die Header von dort einbinden. Ausserdem wäre das nur eine Übergangslösung, bis die Default-Compiler der Distributionen das unterstützen (wenn alles gut geht müsste man dann nur das #include <fmt/format.h> in ein #include <format> und den Namespace bei den Aufrufen von fmt:: nach std:: abändern).



  • Doch, es müssen selbstverständlich diese Objekte instanziiert werden, nur eben direkt und nicht als Zeiger:

    Display display(...);
    Apple apple(...);
    

    Aber du hast noch so viele Design- und Codefehler, daß es wirklich besser wäre, dieses Projekt noch mal neu aufzusetzen.
    Hier eine (kurze) Liste der Verbesserungen:

    • Projektaufbau nach dem 3-Schichten-Modell: UI, Logik und Datenzugriff (Kurzer Überblick, wenn auch mit Java-Code: Wie funktioniert die 3-Schichten-Architektur?), d.h. die Klassen so organisieren, daß sie zu genau einer der Schichten gehören.
    • Vermeidung von direktem Zugriff auf tiefere Objekte (Daten), wie z.B. Apple->ui->display->xres, d.h. Beachtung des Gesetz von Demeter. Kennen sollte man auch die anderen Prinzipien des Artikels (zumindestens von SOLID sollte man schon gehört haben und diese bei OOP auch beachten).
    • Nur die Header je Datei einbinden, die auch direkt benötigt werden - und nicht, wie in "_Display.h", alle Systemheader einbinden! In Headerdateien reichen auch meistens Vorwärtsdeklarationen.

    Insgesamt ist dein Code auch viel zu lang (für die eigentliche Funktionalität), d.h. es erscheint mir eher eine Copy&Paste-Programmierung zu sein, anstatt Aufteilung in passende (kleinere) Funktionen.

    Und dann kommt hinzu, daß eine Umsetzung von C nach C++ nicht bloß das Austauschen von Funktionen bzw. Typen ist (dies wird oft als "C mit Klassen" bzw. "C mit cout" bezeichnet), sondern bei (modernem) C++ werden auch andere Techniken eingesetzt, z.B.



  • @Schlangenmensch @Finnegan @Th69
    Ich hab es ohne "new" compiliert bekommen, aber es läuft sofort in einen Speicherfehler.



  • @Th69
    Copy and Paste ist mein Mittel der Tippfehlervermeidung!
    Und Die Paradigmen Anderer zur Umsetzung eigener Programme ist sinnvoll wenn man die Paradigmen schon kennt, man sucht doch aber nicht nach solchen sondern wird kreativ.
    Sicher sind Funktionen des Programms optimierbar, aber das sind (mir) im moment "Kinkerlitzchen"



  • @EL-europ sagte in Klassenfrage 🙂:

    @Schlangenmensch @Finnegan @Th69
    Ich hab es ohne "new" compiliert bekommen, aber es läuft sofort in einen Speicherfehler.

    Die Ursache dafür wirst du schon selbst mit den genannten Werkzeugen herausfinden und beheben müssen, da dein Programm nicht nur ein Fraktal generieren möchte, sondern obendrein scheinbar auch noch fraktale Fehler erzeugt ( 😉 ), die es sehr schwer machen für jemanden, der nicht genau dasselbe Setup fährt, diesen Fehler nachzuvollziehen (siehe unten, mein Test unter WSL2). Eine IDE mit einem integrierten Debugger ist da übrigens ganz hilfreich - Puristen können gdb auch über die Kommandozeile verwenden (davon weiss ich aber nicht allzu viel).

    Wenn du das tatsächlich mit deinem aktuellen Code weiter durchziehen willst, solltest du Sanitizer und Warnungen aktivieren (inklusive -Werror), die auftretenden Fehler einen nach dem anderen ausfindig machen, herausfinden weshalb sie auftreten und die Ursache beseitigen - und dabei natürlich überlegen ob und wie man es vielleicht besser machen könnte. Anders sehe ich nicht, wie du mit dem Programm auf einen grünen Zweig kommen kannst.

    Einen Speicherfehler bekomme ich beim Testen übrigens auch, aber sehr wahrscheinlich aus völlig anderen Gründen wie bei dir. Bei mir unter Windows mit WSL2 existiert z.B. das Framebuffer-Device (/dev/fb0) nicht, daher gibt open("/dev/fb0", O_RDWR) in Display.cpp bei mir -1 zurück und strerror(errno) den String No such file or directory.

    Leider pfüfst du danach nur, ob fbfile ungleich 0 ist, was du als "kein Fehler" interpretierst (siehe Doku zu open, meiner Ansicht nach ist nicht nur alles was kleiner als 0 ebenfalls ein Fehler, sondern auch noch 0 ein gültiger File-Deskriptor, also kein Fehler) und dein Code macht munter weiter als wäre nichts gewesen 😉

    Auch mit Fehlerbehandlungen wie in solchen Zeilen:

    if (ioctl(fbfile, FBIOGET_FSCREENINFO, &Finfo)){return;}
    if (ioctl(fbfile, FBIOGET_VSCREENINFO, &Vinfo)){return;}
    

    Damit machst du dir und anderen nur das Leben schwer, da hat man praktisch keine Chance als Nutzer der Display-Klasse herauszufinden, ob die Initialisierung nun erfolgreich war oder nicht - geschweige denn warum sie überhaupt fehlgeschlagen ist.

    Eine Möglichkeit, solche Fehler sauber zu handhaben wären Exceptions. Das könnte dann z.B. so oder ähnlich aussehen:

    _Display.cpp

    ...
    #include <string>
    #include <cstring>
    #include <stdexcept>
    ...
    
    Display::Display()
    {
        ...
        
        fbfile = open("/dev/fb0", O_RDWR);
        if (fbfile < 0)
            throw std::runtime_error{
                std::string{ "Unable to open '/dev/fb0': " } + std::strerror(errno)
            };
    
        ...
    
        if (
            ioctl(fbfile, FBIOGET_FSCREENINFO, &Finfo) < 0
            || ioctl(fbfile, FBIOGET_VSCREENINFO, &Vinfo) < 0
        )
            throw std::runtime_error{
                std::string{ "Failed to acquire framebuffer screen information for '/dev/fb0': " }
                + std::strerror(errno)
            };
    
    ...
    

    mandel.cpp:

    ...
    #include <iostream>
    #include <stdexcept>
    ...
    int main()
    {
        try
        {
            ...
            //Programmende
            return 0;
        }
        catch (const std::exception& e)
        {
            std::cerr << "ERROR: " << e.what() << std::endl;
            return 1;
        }
        catch (...)
        {
            std::cerr << "Unexpected exception!" << std::endl;
            return 2;        
        }
    }
    

    Edit: Exceptions sind natürlich noch ein Grund mehr auf manuelle Speicherverwaltung zu verzichten. Stichwort Exception Saftety. Bezogen auf die Display-Klasse könnte man fbspiegel wahrscheinlich als std::vector<uint8_t> umsetzen und das Memory Mapping (mmap/munmap) über einen std::unique_ptr<uint8_t> mit Custom Deleter erschlagen.

    Z.B. so oder ähnlich (ungetestet):

    #include <memory>
    
    class Display
    {
        ...
        std::unique_ptr<std::uint8_t> fbpointer;
        ...
    };
    
    Display::Display()
    {
        ...
        fbpointer = std::unique_ptr{
            static_cast<std::uint8_t*>(
                mmap(
                    0, 
                    bufferlength, 
                    PROT_READ | PROT_WRITE, 
                    MAP_SHARED, 
                    fbfile,
                    0
                )
            ),
            [&](std::uint8_t* ptr){
            {
                munmap(ptr, bufferlength);
            }
        };
        ...
    }
    

    Der std::unique_ptr ruft nämlich automatisch den Deleter (hier die Lambda-Funktion) auf, wenn er zerstört wird. Auch dann, wenn Display nicht vollständig konstruiert wurde.

    Das Problem, wenn im Konstruktor eine Exception fliegt ist nämlich, dass zwar für alle bereits konstruierten Member-Variablen die Destruktoren aufgerufen werden, nicht aber für Display selbst. Das macht es schwierig, mit manuellem malloc/free oder map/unmap den Konstrukor exception-sicher hinzubekommen, so dass die bis dahin reservierten Ressourcen auch alle wieder freigegeben werden.

    Die Lösung ist hier RAII zu verwenden, z.B. mit dem genannten std::vector und dem std::unique_ptr. Dann klappts auch, wenn die Ausnahmen fliegen 😉



  • @Finnegan
    Großen Dank für deine Hinweise, vor allem try/catch is teine gute Idee (den Rest muss ich noch genauer betrachten). Erfahrene Programmierer berücksichtigen schon zu Beginn die "Allgemeingültigkeit" ihres Codes, ich bin tatsächlich nicht daran interressiert gewesen (will aber kein unerfahrener bleiben). Das ich das sanitizer Flag nicht bei den Objektdateien setzen, sondern oben im makefile (beim Linker laube ich) hab ich grad rausgefunden.
    Ich war Elektriker der mit windos in Pascal und VB programmiert hatte, bis ich merkte das ich keinen Zugriff auf die Hardware bekomme und auf Linux umstieg; das dauerte Jahre weil kein Anderer (in meinem Umfald) Linux nutzte. Und ich machte direkt den Fehler mit Java (um die Jahrtausendwende) zu beginnen, bis ich merkte von Fremdbibliotheken abzuhängen und von Hardware noch immer sehr weit entfernt bin (es waren ja alles nur Blackboxen für mich). Aber die Bibliotheken waren von Java und MS Büchern geflutet und nix über c (in Deutsch). Ich bin also ein Irrender der sich in die Visualisierung der Mandelbrodtmenge verliebt hat und Filme, mit geilen Beats untermalt, davon machen will. Aber ich bin auch ein Schwamm der aus eigenen und Fehlern Anderer lernt.
    Deine Hinweise zu den std::vector Methoden hab ich umgesetzt bekommen, aber wenn ich ohne "=new" das prog compiliert bekomme läuft es direkt in einen Speicherfehler. Mit dem sanitizer Falg muss ich jetzt erst ma spielen um es nutzen zu können, deshalb werden erst alle Anderen brauchbaren Hinweise umgesetzt. Dank Allen hier, auch den Trollen



  • @EL-europ

    Ja. Stelle jedoch sicher, die Prinzipien der OOP nicht zu verletzen. Um dem gerecht zu werden gäbe es einmal die Möglichkeit der Vererbung und auch Mehrfachvererbung was einen gewissen Verwandtschaftsgrad der Klassen voraussetzt. Lässt sich ein verwandschaftlicher Bezug der Klassen nicht herstellen, ist es möglich, die Methoden fremder Klassen zu delegieren, also quasi zu eigenen Methoden zu machen. Hierzu bekommt die eigene Instanz als Eigenschaft eine Instanz der fremden Klasse (s.u). Das ermöglicht sogar, der eigenen Methode den gleichen Namen derjenigen fremden Methode zu geben die dann aufgerufen wird. Man kann das auch so machen, daß die Instanz der fremden Klasse erst erstellt wird, wenn die Methode der eigenen Klasse aufgerufen wird (lazy delegation).

    Anm.: Aggregation



  • @Finnegan
    fbpointer als std::unique_ptr zu deklarieren funz bei mir nicht, auch wenn ich eine "{"-Klammer hinzufüge oder wegnehme bringt er mir "include/_Display.cpp:43:17: error: class template argument deduction failed:" Das schwerwiegende Problem sind die "Globalen" thr-Variablen in der _Apple Klasse. Denn erst beim Aufbau des Objekts Apple beginnt sanitaze::address abzubrechen, das Ui wird dargestellt der Rest nicht.



  • @_ro_ro
    Dein Post gibt mir schon zu Denken, doch das einzige was dem Programm (abgesehen der Abbrüchen) noch fehlt ist Funktionalität, und die bekomme ich mit dem Desing hin wenn ich doch nur die Thread Funktion in c++ sauber hinbekäme.
    Die unterteilt die x-Achse in gleichgroße Abschnitte und schickt dem Abschnitt entsprechend viele Threads los um die y-Werte mit einer "getIterFuntion" zu berechnen. Ich habs mit dem Wolf und dem Netz versucht, doch die Beschreibungen waren(sind) mir sehr Abstrakt. Jetzt werd ich erst mal zumindes den Rest mit den exceptions umsetzten und versuch heraus zu finden ob das erste catch ausreicht