Klassenfrage :)
-
@Jockelx
Ich weiß nicht was genau die Initalisierungsliste bedeuted, aber diese Variable wird zur Laufzeit berechnet und wird public benötigt. Ich hoffe eure Compiler geben nur Hinweise aus das ich auf diese Variable ein Augenmerk haben soll, denn bei mir "meckert" er das nicht an
-
@EL-europ sagte in Klassenfrage :
Doch danke für deine Hinweise, da hab ich noch ne compilermeldung:
include/_Colorinterface.cpp:245:86: warning: narrowing conversion of ‘(((_Colorinterface*)this)->_Colorinterface::elements.std::vector<_CiElement>::size() - 1)’ from ‘std::vector<_CiElement>::size_type’ {aka ‘long unsigned int’} to ‘int’ [-Wnarrowing]List Inizialization (Initialisierung von
farbe2
in Zeile 245 in geschweiften Klammern) ist etwas penibler was die übergebenen Typen angeht. Diese Warnung bekommst du, weil du einenint
mit einem Ausdruckvector.size() - 1
initialisieren willst, der den Typvector<T>::size_type
hat, was meistens einuint64_t
ist (64 Bit ohne Vorzeichen statt 32 Bit mit Vorzeichen, wie es fürint
meistens der Fall ist).Um die Warnung wegzubekommen, kannst du z.B. den
id
-Member von_farbe
* zu einemstd::size_t
machen oder du castest den Ausdruck explizit in einenint
, wobei du sicherstellen solltest, dass dieser keine Werte außerhalb des Wertebereichs vonint
haben kann und dabei komische Sachen passieren können.Wenn ich versuche das letzte Objekt im std::vector mit der array.end() "-Funktion" anzusprechen, compiliert er mir nicht.
Die
end()
-Iteratoren von Containern verweisen nicht auf das letzte Element, sondern auf ein Element dahinter. Ein Zugriff auf dieses nicht-existierende Element ist Undefined Behaviour.Alternativen sind die Funktionen
vector.back()
odervector.at(vector.size() - 1)
/vector[vector.size() - 1]
, die eine Referenz auf das letzte Element zurückgeben (UB fallsvector
keine Elemente hat),vector.end() - 1
falls es sich um einen Bidirectional Iterator handelt (beivector
-Iteratoren der Fall) odervector.begin() + vector.size() - 1
(was aber nur bei einem Random Access Itetrator funktionieren sollte, das ist aber beivector
ebenfalls gegeben).* Namen die mit Unterstrich gefolgt von einem Kleinbuchstaben beginnen sind in C++ übrigens für die Implementation reserviert, also für den Compiler und die Standardbibliothek. Auch wenn das oft keine Probleme macht, ist es empfehlenswert, solche Namen zu vermeiden.
-
@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 CodesIch 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
new
und keinmalloc
- Du brauchst ein Array -> nutze
std::vector
- Du meinst Pointer zu brauchen -> nutze
std::unique_ptr
oderstd::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)
.
- Nutze kein
-
@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
undstruct
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
keinnew
,malloc
,delete
oderfree
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, womalloc
seine internen Daten verwaltet. Oder du hast irgendwo einen Pointer mitfree
freigegeben, der zuvor nicht mitmalloc
reserviert wurde. Z.B. kann mitmalloc
reservierter Speicher nur mitfree
freigegeben werden und solcher, der mitnew
reserviert wurde nur mitdelete
. 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 deinengcc
-Aufrufen imMakefile
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
undmalloc
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 :
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ürmyApples.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)
-
(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 FlagJa, 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 zulasseDie 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 funktioniertIch 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, besserstruct _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. MItstruct
undclass
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, einstd::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 überpush_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.