sanitizer::address unlogik?

Schlangenmensch

@EL-europ Ganz ehrlich, beherzige die Tipps die du zu genüge bekommen hast.
Das ist nicht nur kein C++, das ist auch schlechter C Code.

iterMembers = (int**)malloc(1 * sizeof(int *));

f(! inarray){
   if((countsOfIter % 199) == 0){
	if( !(iterMembers = (int**)realloc(iterMembers, (countsOfIter +200) * sizeof(int *)))){return;}
		for(int k=0; k<200; k++){
			if( !(iterMembers[countsOfIter+k] = (int*)malloc( (2) * sizeof(int)))){return;}

Wer soll denn da verstehen, wie groß dein iterMembers zu welcher Zeit ist?

Bist du sicher, das countsOfIter häufig genug inkrementiert wird, um hinterher alles frei zu geben.

Noch mal: Ganz ehrlich, schmeiß das weg.
Entweder, mach das in C++ neu.Wenn du dir ins Bein schießen willst und das mit manueller Speicherverwaltung machen willst, und Sachen wie Vektor nachbauen willst, ok, aber google mal RAII.

Wenn du das in C machen willst, kannst du die Speicherverwaltung auch kapseln. Z.b. so ungfähr

typedef struct 
{
   int* values;
   int length;
} MyArray

MyArray* getArray(unsigned n) {

  MyArray* anArray = malloc(sizeof(MyArray));
  if (anArray == NULL) 
    return NULL;
          
  anArray->values = malloc(sizeof(int) * n);
  if (anArray->values == NULL ) {  
    free(anArray );             
    return NULL;                       
  }

  anArray->length= n;
  return anArray ; 
}

void freeArray(MyArray* anArray) {
  if (anArray == NULL) 
    return;               
  
  free(anArray->values); 
  free(anArray );
}

Ich habe bestimmt seit 10 Jahren kein plain C mehr programmiert und ich habe das nicht probiert, daher keinerlei Garantie. Aber so wie du deine Speicherverwaltung machst, ist es kein Wunder, dass dir das um die Ohren fliegt und es ist nur sehr schwer möglich zu sagen, wo genau der Fehler ist.

Das eigentliche Problem ist, dass es mit dem Programmierstil kaum möglich ist, sowas fehlerfrei zu implementieren, egal ob in C oder in C++.

EL europ

@Schlangenmensch Ja, die iterMembers allociere ich in 200er Schritten wegen der Laufzeit. Dort Versuche ich, wenn ich den Fehler nicht finde, eine Version mit std::vector und std::sort(). Ist es möglich mit std::sort() ein zweidimensionales Array naxh x oder y zu sortieren?

EL europ

@Schlangenmensch Den Code von dir oben wer ich mal, aber leider erst nachmittags, versuchen zu verstehen und zu übertragen

Schlangenmensch

@EL-europ std::sort kannst du mitgeben, wie du sortieren möchtest. Ich würde dir die Doku dazu empfehlen: https://en.cppreference.com/w/cpp/algorithm/sort oder, vlt je nach compiler noch etwas einfacher für dich: https://en.cppreference.com/w/cpp/algorithm/ranges/sort

Im Zweifel können wir dir da auch unter die Arme greifen, wenn du soweit bist.

Wenn du wirklich C++ machen willst, dann brauchst du meinen Code nicht, weil das ein std::vector alles schon mit bringt.

EL europ

@Schlangenmensch
Nun ja, es geht mir nicht um c++ sondern um Laufzeit. Aber die hängt an den **thr_... und **iterMembers Funktionen. Vielleicht ist es gar sinvoll die **iterMebers auch über Threads laufen zu lassen.
Das Prog soll mehrere tausend Apfelmänchen in Folge berechnen und als "rohe" Daten auf Platte speichern. Also ist das "Speicherloch" in Apple->sort() untragbar. Es Wäre fatal ne halbe Stunde zu warten um dann einen Abbruch wegen eines Speicherfehlers zu bekommen.

Th69

Wie ich schon schrieb:
@Th69 sagte in Klassenfrage : ):

Du brauchst auch nicht jedesmal wieder neuen Speicher anzufordern, da sich ja xres und yres währenddessen nicht ändert, d.h. bei Umstellung auf vector<...> reicht es diesen einmalig in der passenden Größe zu erzeugen (bzw. direkt in Init(...)).

Und bzgl. Performance kann man deinen Programmcode um einiges noch verbessern, da du viel zu viele unnötige Speicherallokationen und Berechnungen durchführst (und bei einem besseren Design, d.h. Codestruktur, könnte man einiges viel eleganter lösen).

Wenn du wirklich ein performantes Programm erstellen willst, dann laß diese unnötigen Speicherallokationen in jeder Funktion sein, sondern erstelle sie nur bei Bedarf (nur wenn sich die Größe der Bilder ändert o.ä.)! Diese Apfelmännchen werden doch sicherlich alle in derselben Größe erstellt, oder? Und selbst, wenn nicht, brauchst du nicht in jeder Funktion Speicher allozieren und wieder freigeben.

Du solltest wirklich mal eine kurze Programmierpause machen und dir mal ein Konzept (Design) für dein Programm erstellen (z.B. eine Liste der Einzelfunktionalitäten sowie ein Ablaufdiagramm).
Durch die Teilumstellung auf C++ ist dein Code noch undurchschaubarer und quasi unwartbar geworden (u.a. da du, trotz Benutzung von Klassen, OOP nicht sinnvoll anwendest).

Auch hast du meinen Vorschlag (thr_matrix = matrix) genau falschherum umgesetzt!
Die Membervariable matrix muß den Speicher allozieren und dieser wird dann nur an die Threadfunktion weitergereicht, so daß dann auch das Apple-Objekt für die Freigabe zuständig ist (so daß es dann auch bei mehreren Apple-Objekten einwandfrei funktionieren würde - dies ist der Grundgedanke beim Klassendesign).
Aber bei Umstellung auf std::thread bräuchtest du auch keine globalen Variablen mehr und könntest direkt auf matrix zugreifen.

Wenn du aber so, wie die letzten Tage, weitermachst, wirst du dein Programm nie sauber (und performant) umsetzen können.

Was ich dir anbieten kann: wenn du ein Mikro hast, dann könnten wir mal über Discord dies besprechen.

Schlangenmensch

@EL-europ sagte in sanitizer::address unlogik?:

Nun ja, es geht mir nicht um c++ sondern um Laufzeit

Mit ordentlichem C++ wirst du eine sehr gute Laufzeit hinbekommen. Aber, du musst dich für eine Programmiersprache entscheiden: C oder C++.

Für performante Software, unabhängig von der Programmiersprache:

Schritt 1) Eine ordentliche Software schreiben (saubere Algorithmen, Teile mit Unittests getestet, kommentiert)
Schritt 2) Mit Optimierungsflag gesetzt kompilieren
Schritt 3) Performance messen

Wenn das dann nicht performant genug ist, dann kann man anfangen manuell zu optimieren. Wenn man das direkt "mitdenken" will, kann man darauf achten:

Wenige Speicherallokationen
Wenig "rum kopieren" von großen Speicherblöcken
Daten linear im Speicher halten (kein Array von array, sondern am Stück und mit den richtigen Indizes drauf zugreifen)

Wenn das nicht reicht, kann man gucken, ob man das sinnvoll parallelisieren kann. Aber, es wäre nicht das erste mal, dass das Erstellen von Threads teurer ist, als der performance Vorteil durch die Parallelisierung.

wob

Dieser Beitrag wurde gelöscht!

EL europ

@Th69 Die Größe der Apfelmänchen ist variabel und kann per ui geändert werden, aber das ist ja "einfache" Funktionalität. Der *thr_Function muss ich noch Parameter hinzufügen um die Farbgebung zu beeinflussen. Aber das ist noch "Zukunftsmusik".

EL europ

@Schlangenmensch
Das "leak" in sort hab ich auch weg bekommen mit:
std::vector<std::array<int, 2>> iterMembers

firefly

@EL-europ sagte in sanitizer::address unlogik?:

@Schlangenmensch
Das "leak" in sort hab ich auch weg bekommen mit:
std::vector<std::array<int, 2>> iterMembers

Für was das std::array der größe 2? Wäre es da nicht besser eine struct mit zwei member zu haben?
Dadurch ist klarer was was ist.

EL europ

@firefly
Das ist Kosmetik denke ich?

firefly

@EL-europ sagte in sanitizer::address unlogik?:

@firefly
Das ist Kosmetik denke ich?

Ist dir überlassen wenn du immer rätseln musst ob array member 0 oder 1 jetzt das korrekte Datum darstellt.

Und das rätseln wird definitv kommen wenn du mal länger nicht an dem Projekt gearbeitet hast und dann daran zurückkehrst.
Aber vermutlich war mein Hinweis hier vergeudete Zeit wenn ich mir so deine Reaktionen auf gut gemeinte Hinweise hier in diesem Thread und dem anderen so anschaue

EL europ

Ich will einen std::vector<MemApple> in ein File speichern:

struct MemApple{
	struct ApplePara paras;
	int pic[100][45];
	std::vector<struct _CiElement> ci_elements;
};

Bei Folgendem Code:

void AppleMemory::writeToFile(){
	std::ofstream file("mem.Apples", std::ios::out|std::ios::binary);
	int count = memApples.size();
	file.write( (char*)(&count), sizeof(int));
	for(int i=0; i<memApples.size(); i++){
		int elem_size = memApples.at(i).ci_elements.size();
		file.write( (char*)(&elem_size), sizeof(int));
		file.write(reinterpret_cast<char*>(&memApples[i]), sizeof(memApples[i]));
	}
	file.close();
}
AppleMemory::AppleMemory(int x, int y, Colorinterface &_ci){
	xpos = x;
	ypos = y;
	ci = &_ci;
	std::ifstream file("mem.Apples", std::ios::in|std::ios::binary);
	if(!file.fail()){
		int count = 0;
		file.read((char*) &count, sizeof(int));
		memApples.resize(count);
		for(int i=0;i<count; i++){
			int size = 0;
			file.read((char*) &size, sizeof(int));
			memApples.at(i).ci_elements.resize(size);
			file.read(reinterpret_cast<char *> (&memApples[i]), sizeof(memApples[i]));
		}
		file.close();
		drawApples();
	}
}

erkennt der leak-sanitizer fehler mit der meldung "new Operator..." bezüglich der ci_elements. Wo ist hier der Fehler?

EL europ

@firefly Ha wenn ich eine Woche meinen Code nicht anschaue, vergesse ich ihn komplett. Anstatt der Kosmetik ist die Kommentierung wichtiger, da werden dann auch leichte kosmetische Änderungen durchgeführt. Doch die iterMembers ist so was wie die dritte Achse, ich muss nur als schauen an welcher Positiion ich welchen der beiden Werte speicherte.

Wenn die Klasse AppleMemory steht, kann ich selbst mit dem Prog arbeiten um ein Verfahren zur Farbgebung zu finden das dann "implementiert" wird.

Ich programmiere ja weil ich Daten errchnet haben will und nicht des Programmierens wegen, bin also mehr User wie Programmierer. Und die Ratschläge hier nehme ich mir schon zu Herzen, Nur ist eine Antwort darauf, oder umsetzung im Code oft nicht unmittelbar möglich

Schlangenmensch

@EL-europ
Ich habe schon mal gesagt, dass du die ganzen struct nicht brauchst, das ist C++ kein C

struct MemApple{
	ApplePara paras;
	int pic[100][45];
	std::vector<_CiElement> ci_elements;
};

Hast du Testfälle für deinen Code? Ich kann mir spontan nicht vorstellen, dass das Speichern und Laden so funktioniert. sizeof kann ja nicht wissen, wie groß der Vektor ci_elements ist. Aber sowas kann man prima mit Unittests überprüfen.

Ansonsten sehe ich kein new in dem zitierten Code, was erstmal ein gutes Zeichen ist. Insgesamt passiert mit ci_elements nichts in dem gezeigten Code.

@EL-europ sagte in sanitizer::address unlogik?:

Ha wenn ich eine Woche meinen Code nicht anschaue, vergesse ich ihn komplett. Anstatt der Kosmetik ist die Kommentierung wichtiger, da werden dann auch leichte kosmetische Änderungen durchgeführt.

Naja, Kommentare hast du ja keine. Außerdem werden Kommentare schnell vergessen, wenn man was im Code ändert. Am Besten ist der Code selbst Ausdrucksstark, zusätzlich zu Kommentaren, die dann erklären, warum man das so macht.

@EL-europ sagte in sanitizer::address unlogik?:

Ich programmiere ja weil ich Daten errchnet haben will und nicht des Programmierens wegen, bin also mehr User wie Programmierer

Aber du hast Fragen an uns, weil dein Code so nicht funktioniert. Wenn der Code ausdrücken würde, wofür er gedacht ist, könnten wir schneller sehen, wo die Probleme liegen. Außerdem würdest du selbst die Probelem vielleicht schneller sehen.

EL europ

@Schlangenmensch
Ich speichere einen std::vector<MemApple> mit writeToFile() in einem File. Wenn ich die MemApple ohne "membervariable?"std::vector<ci_elements" speichere, "deserialisiert?" er mir sie Folgerichtig und fehlerfrei (also auch nicht im struct deklriere, auch die int[x][x]-arrays funzen). Wenn ich aber den std::vector<ci_elements> deserialisiere bringt er mir die Fehlermeldung, und ein Zugriff auf den deserialisierten std::vector<ci_elements> einen Adressfehler-abbruch.

Also: Wenn ich den vector im vector nach dem gleichen Verfahren deserialisiere, funktioniert das wieder-einlesen nicht richtig.

Schlangenmensch

@EL-europ Ich habe ja bereits geschrieben, dass ich mir nicht vorstellen kann, dass das so funktioniert.
Bei pic ist ja klar, wie große das ist. Das ist schon zur Compilezeit bekannt und das liegt auf dem Stack.

Den Vektor von MemApple versuchst du ja auch nicht, am Stück zu speichern.

MIr geht es um diese Zeile:

file.write(reinterpret_cast<char*>(&memApples[i]), sizeof(memApples[i]));

Je nachdem wie die ci_elements aussieht, könnte das sowas sein, für den Vektor. Da fehlt dann aber noch der Rest des Structs.

for(int j = 0; j <elem_size; j++)
{
 file.write(reinterpret_cast<char*>(&memApples[i].ci_elements[j]), sizeof(memApples[i].ci_elements[j]));
}

Tendentiell würde ich auch Anfängern nicht dazu raten, sowas binär abzulegen. Textdokumente kann man besser manuell debuggen.

EL europ

@Schlangenmensch Textdokumente? Ich will hier "Unmengen" an Integerwerten speichern! Das speichern und einlesen eines vektors funktioniert auch, (stand auf github). Nur wenn die elemente selbst wieder vectoren enthalten, funktioniert der code nicht.
Du meinst also, der vector im vector muss selbst element für element geschrieben und gelesen werden? und wenn ja, in welcher Reihenfolge muss dann wieder eingelesen werden um den vector "ganz" zu bekommen?

Schlangenmensch

@EL-europ Beziffer Unmengen. Mach Tests mit kleineren Mengen. Wie gesagt, erstmal textbasiert, damit du verstehst was da passiert. Mach automatisierte Tests. Das muss ja alles nicht am lebenden Objekt gebastelt werden.

Natürlich kann man Vektoren speichern und lesen, nur halt nicht so, wie du es versucht hast.

Beim Einlesen musst du erst die gespeicherte Anzahl an Elementen lesen und dann die Elemente einlesen. So wie du die auch speichern solltest.
Beim Einlesen von memApples machst du das doch prinzipiell auch.