Klassenfrage :)



  • @Finnegan
    Danke, die compiler-Meldung hab ich mit einem (int) cast wegbekommen und die Bereichsüberschreitung ist leicht in den Griff zu bekommen. Das mach ich jetzt... und der Hinweis mit den Unterstrichen, danke das wird Berücksichtigung finden 🙂



  • Ich hab zur Laufzeit manchmal Abbrüche mit der Meldung "malloc(): invalid size (unsorted)". Denke das kommt von den c-arrays?
    Aber die thread Funktion bekomm ich in c++ ohne ein neues Buch nicht hin, und sie ist mein ganzer stolz 🐕 (Und ich muss den ganzen Hirnschmalz noch mal investieren) 😞

    ist es nicht schlimm schon Gedachtes nochmals denken zu müssen weil man Details vergessen hat? Das Alter!



  • @EL-europ sagte in Klassenfrage 🙂:

    @Quiche-Lorraine
    Da triffst du den Nagel zumindest nah am Kopf:
    Ich allociere in einer Klasse c-arrays, wenn ich Objekte dieser Klasse in std::vector speichere und diese Arrays im Destruktor mit free() freigebe, funktionieren entsprechende Methoden der Objekte in std::vector nicht mehr. Ich geb zu das die c-arrays und c-threads "quick n dirty" Lösungen sind weil ich meine thread-Lösung nicht einfach in c++ threads umgesetz bekam, wie den Rest des Codes

    Ich würde darauf wetten, dass die Klasse, die selbst Speicher alloziert kaputt ist und du die Rule of five nicht befolgst. Soll heißen, deine Klasse verhält sich anders, wenn Kopien von ihr angelegt werden, als du denkst.
    Da ein Vektor mit wächst, kann es sein, dass die in ihm gespeicherten Objekte kopiert werden müssen.

    Solange du keine GUI Library verwendest (gerade für GUIs gibt es da schonmal ausnahmen) würde ich darauf wetten, das du hervorragend ohne new auskommst.

    • Nutze kein newund kein malloc
    • Du brauchst ein Array -> nutze std::vector
    • Du meinst Pointer zu brauchen -> nutze std::unique_ptr oder std::shared_ptr(bevorzugt ersteres)
      Nutze keine globalen Variablen. Wenn du ein Objekt der Klasse wo anders brauchst, übergeb es als Parameter.

    Wenn du die ersten drei Punkte befolgst, brauchst du wahrscheinlich keinen selbstgeschriebenen Destruktor mehr. Außerdem funktionieren Sachen, wie Objekte kopieren einfach so von sich aus richtig, oder lassen sich nicht kopieren (wenn ein std::unique_ptr als Member verwendet wurde)
    .



  • @Schlangenmensch
    Ich hab die Objekte der Klasse nicht mehr in std::vector, sonder "initialisiere" ein einziges Objekt mit anderen Parametern, und speichere die Parameter als struct in einem std::vector. Ich hoffte das die c-arrays damit laufen würden (ich vermute nur die c-arrays sind das prob)



  • @EL-europ Der Einzige Unterschied zwischen class und struct in C++ ist die default Sichbarkeit der Member.
    Ich vermute, dass dein Code einige Probleme haben wird, das zeigt einfach die Erfahrung wenn Leute von C mal schnell auf C++ wechseln (und insbesondere, wenn sie C schon nicht so richtig gelernt haben).

    Wenn du in deinem struct kein new, malloc, delete oder free hast, ist die Chance gut, das das in dem Vektor funktioniert.

    Aber, ob und wo du Probleme in deinem Code hast, oder haben könntest, wird man dir nur sagen können, wenn du ihn hier mit uns teilst.





  • @EL-europ sagte in Klassenfrage 🙂:

    malloc(): invalid size (unsorted)

    Das riecht sehr stark nach einer beschädigten Datenstruktur der malloc-Implementierung. Eventuell schreibt dein Programm in Speicherbereiche, wo malloc seine internen Daten verwaltet. Oder du hast irgendwo einen Pointer mit free freigegeben, der zuvor nicht mit malloc reserviert wurde. Z.B. kann mit malloc reservierter Speicher nur mit free freigegeben werden und solcher, der mit new reserviert wurde nur mit delete. Die Mechanismen sind nicht untereinander austauschbar.

    Da du mit GCC kompilierst, würde ich dir mal empfehlen, dein Programm mal mitein paar aktivierten Sanitizern (-fsanitize=...) und Debug-Informationen (-g, für Infos zu Quellcode-Datei und Zeilennummern) zu bauen. Z.B. indem du folgede Parameter zu deinen gcc-Aufrufen im Makefile hinzufügst:

    -g -fsanitize=address -fsanitize=leak -fsanitize=undefined

    Clang kennt diese Sanitizer übrigens auch - sogar noch ein paar mehr, -fsanitize=memory (entdeckt Verwendung von nicht-initialisiertem Speicher) wird z.B. von GCC (noch) nicht unterstützt.

    Mit diesen aktivierten Sanitizern steigt dein Programm dann aus, wenn ein Buffer Overflow, Use-After-Free, ein Speicherleck oder ein Undefined Behaviour-Konstrukt gefunden wird. Mit zusätzlichen Informationen wo im Code das Problem entdeckt wurde.

    Die Sanitizer garantieren nicht, dass jedes Problem gefunden wird, aber sie finden schon eine ganze Menge und sind ein sehr nützliches Werkzeug, das direkt bei deinem Compiler dabei ist. Es lohnt sich, sich mal mit denen zu beschäftigen - es gibt auch noch andere nüzliche Sanitizer.

    Ansonsten nimm dir auch die Empfehlungen von @Schlangenmensch und anderen hier zu Herzen. Wenn du ohne new und malloc auskommst oder zumindest Regeln wie die Rule of 3/Rule of 5 sorgfältig berücksichtigst, wirst du diese Art von Problemen wahrscheinlich gar nicht erst bekommen.



  • @EL-europ sagte in Klassenfrage 🙂:

    @Schlangenmensch
    https://github.com/momefilo/mandelbrodt

    Last commit von vor 17h.

    Ich sehe im Toplevel cpp gerade das hier:

    void newApple(){
    		myApples.push_back(*new _AppleData);
    		myApples.at(aktApple).xres = Apple->xres;
    		myApples.at(aktApple).yres = Apple->yres;
    		myApples.at(aktApple).depth = Apple->depth;
    		myApples.at(aktApple).rmin = Apple->rmin;
    		myApples.at(aktApple).rmax = Apple->rmax;
    		myApples.at(aktApple).imin = Apple->imin;
    		myApples.at(aktApple).imax = Apple->imax;
    		myApples.at(aktApple).xpos = Apple->xpos;
    		myApples.at(aktApple).ypos = Apple->ypos;
    		myApples.at(aktApple).width = Apple->width;
    		myApples.at(aktApple).height = Apple->height;
    		aktApple++;
    }
    

    a) Du solltest keine globalen Variablen verwenden (weder myApples noch Apple).
    b) Das ist doch einfach nur der Copy-Constructor. Was soll dieser Code?! Komplizierte Schreibweise für myApples.push_back(Apple)?

    Und ich persönlich würde Variablen immer klein schreiben, Klassen/Structs groß. Dein Apple ist ja eine Variable. Der Typ ist _Apple*. Brr. Underscore & pointer weg! Und was soll diese Duplizierung von _Apple und _AppleData? (ah, ist vielleicht doch kein Copy-Constructor, sieht aber so aus, als sollte es einer sein, weil du 2 Klassen mit fast demselben Inhalt hast)


  • Gesperrt

    (Das Forum harkt gerade extrem bei mir...)

    Man schaue auch mal nach den anderen Madelbrot-Themen in diesem Forum ... oder geht es darum gar nicht?



  • @wob
    diese struct _AppleData speichert nur die Parameter-Sätze mit denen das einzige Objekt der Klasse _Apple immer wieder neu initialisiert wird.
    Es gibt vier Klassen von denen allen zur Laufzeit jeweils nur ein Objekt mit new instaziert wird.
    Colorinterface->Apple->Userinterface->Display.
    Ein einziges Objekt der Klasse "_Display" wird einem einzigen Objekt der Klasse " _Userinface" übergeben, dieses Userinterface-Objekt wird einem einzigen Objekt der Klasse _Apple übergeben welches slebst dem einzigen Objekt der Klasse "_Colorinterface" Übergeben wird.

    Ich "liebe" diese Mandelbrotbilder und hab mit meiner ersten Python version diesen Programms und ffmpeg, Filme mittels Bildfolge der einzelnen "Farb-iterationsstufen" erstellt; Die sind Großformatfüllend betrachtet, berauschend.
    Um solch eine Funktionalität nutzerfreundlich zu realisieren ist dieses mandelbrodt gedacht.
    Ich habe Start- und Endparameter solch einer Bildsequenz vor meinem geistigen Auge, welche die Berechnung steuern. Muss aber alles erst noch Programmiert werden, bis jetzt ist ja nur möglich in das Apfelmänchen zu zoomen und es einzufärben und dann zu betrachten. (Von den spontanen Abstürzen abgesehen, die ich erst weg haben will bevor Funktionalität hinzugefügt wird)



  • @omggg
    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



  • @Finnegan
    Verstehe ich das richtig, das -fsanitize mir Meldungen(Abstürze) erst zur laufzeit "verursacht" ?
    Ah blöde Frage, eben seh ich das er es gar nicht kompiliert mit diesem Flag



  • Findet sich hier vielleicht ein Kenner in c++ der mir die einfachste Klasse "_Display" mit ihren drei kleinen Methoden in sauberem c++ darlegen kann, als minimal-Bsp-Code in Betracht ihrer Beziehung Colorinterface->Apple->userinterface->Display. Dann kann ich den Rest nach diesem Muster neu gestalten. Sonst muss ich durch den "Wolf" weil mir grad die Mittel fehlen für ein neues Fachbuch



  • @EL-europ sagte in Klassenfrage 🙂:

    @Finnegan
    Verstehe ich das richtig, das -fsanitize mir Meldungen(Abstürze) erst zur laufzeit "verursacht" ?
    Ah blöde Frage, eben seh ich das er es gar nicht kompiliert mit diesem Flag

    Ja, die sanitize-Optionen sind zur Laufzeit.

    Versuchs mal mit clang-tidy oder cppcheck (wobei das Herumdoktoren an den Symptomen wäre, neu schreiben erscheint hier als plausible Option).

    /tmp/mandelbrodt/include/_Apple.cpp:65:66: error: arithmetic on a pointer to void [clang-diagnostic-error]
                            pthread_create( &thrIds[aktThr], NULL, &thrFunc, (void *)tmp_x+aktThr*sizeof(int));
                                                                             ~~~~~~~~~~~~~^
    /tmp/mandelbrodt/include/_Apple.cpp:149:2: warning: 'sprintf' will always overflow; destination buffer has size 17, but format string expands to at least 18 [clang-diagnostic-fortify-source]
            sprintf(iterText, "I-Werte % 8d", countsOfIter);
            ^
    /tmp/mandelbrodt/include/_Apple.cpp:171:3: warning: 'sprintf' will always overflow; destination buffer has size 16, but format string expands to at least 18 [clang-diagnostic-fortify-source]
                    sprintf(text[5], "Itr2= % 10d", iterMembers[countsOfIter-2][0]);
                    ^
    /tmp/mandelbrodt/include/_Colorinterface.cpp:53:2: warning: 'sprintf' will always overflow; destination buffer has size 8, but format string expands to at least 9 [clang-diagnostic-fortify-source]
            sprintf(text,"%- 7d",id);
            ^
    /tmp/mandelbrodt/include/_Colorinterface.cpp:55:2: warning: 'sprintf' will always overflow; destination buffer has size 8, but format string expands to at least 9 [clang-diagnostic-fortify-source]
            sprintf(text2,"%- 7d",satzcount);
            ^
    /tmp/mandelbrodt/include/_Colorinterface.cpp:88:2: warning: 'sprintf' will always overflow; destination buffer has size 9, but format string expands to at least 10 [clang-diagnostic-fortify-source]
            sprintf(text,"% 8d",elements.at(apos).id);
            ^
    /tmp/mandelbrodt/include/_Colorinterface.cpp:102:2: warning: 'sprintf' will always overflow; destination buffer has size 9, but format string expands to at least 10 [clang-diagnostic-fortify-source]
            sprintf(text2,"% 8d",members);
            ^
    /tmp/mandelbrodt/include/_Colorinterface.cpp:245:63: error: non-constant-expression cannot be narrowed from type 'unsigned long' to 'int' in initializer list [clang-diagnostic-c++11-narrowing]
            struct _farbe farbe2{elements.at((elements.size()-1)).color, (elements.size()-1)};
                                                                         ^~~~~~~~~~~~~~~~~~~
    /tmp/mandelbrodt/include/_Colorinterface.cpp:245:63: note: insert an explicit cast to silence this issue
            struct _farbe farbe2{elements.at((elements.size()-1)).color, (elements.size()-1)};
                                                                         ^~~~~~~~~~~~~~~~~~~
                                                                         static_cast<int>(  )
    /tmp/mandelbrodt/include/_Userinterface.cpp:135:2: warning: 'sprintf' will always overflow; destination buffer has size 30, but format string expands to at least 31 [clang-diagnostic-fortify-source]
            sprintf(text[1], "i = %.24f", i);
            ^
    /tmp/mandelbrodt/include/_Userinterface.cpp:152:33: warning: 'sprintf' will always overflow; destination buffer has size 8, but format string expands to at least 10 [clang-diagnostic-fortify-source]
            if(element < 2 || element > 5) sprintf(text, "% 8d", (int)elements.at(element).wert);
                                           ^
    /tmp/mandelbrodt/include/graphics/font8x8.h:67:25: error: constant expression evaluates to 255 which cannot be narrowed to type 'char' [clang-diagnostic-c++11-narrowing]
        { 0x00, 0x66, 0x3C, 0xFF, 0x3C, 0x66, 0x00, 0x00},   // U+002A (*)
                            ^~~~
    /tmp/mandelbrodt/include/graphics/font8x8.h:67:25: note: insert an explicit cast to silence this issue
        { 0x00, 0x66, 0x3C, 0xFF, 0x3C, 0x66, 0x00, 0x00},   // U+002A (*)
                            ^~~~
                            static_cast<char>( )
    /tmp/mandelbrodt/include/graphics/font8x8.h:120:49: error: constant expression evaluates to 255 which cannot be narrowed to type 'char' [clang-diagnostic-c++11-narrowing]
        { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFF},   // U+005F (_)
                                                    ^~~~
    /tmp/mandelbrodt/include/graphics/font8x8.h:120:49: note: insert an explicit cast to silence this issue
        { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xFF},   // U+005F (_)
                                                    ^~~~
                                                    static_cast<char>( )
    

    PS: Warum checkst du die compilierte Datei mit ins Repo ein?



  • Ich nutz das git "nur" als Backup, bei mir sterben die usb sticks, sd-karten und andere speichermeiden manchmal.
    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?
    Die zweite meldung interpretiere ich so, das er den Rest des Strings einfach abschneidet? Aber lass mich belehren und werde die c++ Version std::format nutzen müssen.
    Die Folgende Meldung "narrowed from type 'unsigned long'": bedeuted (vermute ich) nur das ein Long Int zu Int verengt wird, das "prob" umgehe ich in dem ich nur Werte bis 9999999 im prog zulasse

    Die nächste Meldung welche die char-Fontarrays betrifft: Wäre es besser den Typ uint8_t zu nehmem? Nur weiss ich grad net obs dann props mit Funktionen auf ihnen gibt.



  • Gerade die Display ist undankbar, weil System abhängig. Ich sitze z.B. gerade an einem Windows Rechner und probiere zumindest gerne aus, ob das funktioniert 😉

    Ich versuche mal ein bisschen was, anhand der mandel.cpp zu erklären.

    #define BORDER 2
    

    hier würde man in modernem C++

    constexpr unsigned int 2
    

    nehmen. Der Vorteil zur Preprozessor Ersetzung ist, dass das typsicher ist.

    _Display *Display;
    _Userinterface *Ui;
    _Apple *Apple;
    _Colorinterface *Ci;
    

    Wie schon mehrfach geschrieben, globale Variablen nutzt man nicht. Da weiß man nie genau welche in welchem Zustand ist. Da definieren, wo sie gebraucht werden und u.a. als Parameter weiter reichen.

    in _AppleData sind ein paar Member nicht default initialisiert, besser

    struct _AppleData{
    	int xpos{};
    	int ypos{};
    	int width{};
    	int height{};
    	int xres{1920};
    	int yres{1080};
    	long double rmin{-1};
    	long double rmax{2};
    	long double imin{-1};
    	long double imax{1};
    	int depth{100};
    };
    
    std::vector<struct _AppleData> myApples;
    

    das Wort struct brauchst du hier nicht. MIt struct und class definierst du Typen, das hier reicht:

    std::vector<_AppleData> myApples;
    

    Das hier

    int aktApple = 0;
    

    ist irgendwie der Index nach dem letzten Element in myApples. Das brauchst du nicht, ein std::vector kapselt alles, was du für den Umgang damit benötigen könntest.

    Dein _Apple scheint teilweise die gleichen Member zu haben, wie _AppleData. Gib _Apple doch einen Member _AppleData appleData stattdessen und eine Funktion

    _AppleData _Apple::GetAppleData()
    {
       return appleData;
    }
    

    Jetzt wird es wild

    void newApple(){
    		myApples.push_back(*new _AppleData);
    

    new _AppleData erzeugt einen _AppleData auf dem Heap und gibt einen Pointer darauf zurück. Den dereferenzierst du mit * und rufst über push_back impliziet den Copy Ctor auf. Du merkst die aber den Pointer nicht, der auf den Heap zeigt und kannst den Speicher gar nicht mehr freigeben.

    Wenn du es so behalten möchtest, wie du es gerade hast, einfach:

    void newApple(){
    		myApples.push_back(_AppleData());
    

    Wenn du den vorgeschlagenen Getter von mir implementiert hast:

    void newApple(){
    		myApples.push_back(Apple->GetAppleData());
    }
    

    und die Funktion ist fertig.

    Hier

    void ui_callback(int i){
    	if(i<1){
    		myApples.at(aktApple-1).xres = Apple->ui->getWert(0);
    

    willst du auf das letzte element von myApples zugreifen.

    Ohne Indexzugriff und extra merken von aktApple:

    void ui_callback(int i){
    	if(i<1){
    		myApples.back().xres = Apple->ui->getWert(0);
    

    Das hier:

    Apple->xres = myApples.at(aktApple-1).xres;
    			Apple->yres = myApples.at(aktApple-1).yres;
    			Apple->xpos = myApples.at(aktApple-1).xpos;
    			Apple->ypos = myApples.at(aktApple-1).ypos;
    			Apple->width = myApples.at(aktApple-1).width;
    			Apple->height = myApples.at(aktApple-1).height;
    			Apple->rmin = myApples.at(aktApple-1).rmin;
    			Apple->rmax = myApples.at(aktApple-1).rmax;
    			Apple->imin = myApples.at(aktApple-1).imin;
    			Apple->imax = myApples.at(aktApple-1).imax;
    			Apple->depth = myApples.at(aktApple-1).depth;
    

    Sieht so aus, als ob du nicht nur einen Getter auf einen _AppleData Member in Apple brauchen könntest, sondern auch einen Setter:

    void _Apple::SetAppleData(const _AppleData& data)
    {
       appleData = data;
    }
    

    Dann fällt das oben in dashier zusammen:

    Apple->SetAppleData(myApples.back());
    

    Erstmal als Anregung. Ich muss jetzt noch ein bisschen selbst arbeiten 😉

    P.S: Mach die _ vor deinen Typen weg. Die stören doch nur beim Tippen.



  • @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?