Wo leake ich hier memory? Dynamic array

new[] braucht kein sizeof.

hustbaer

@marcel91200 sagte in Wo leake ich hier memory? Dynamic array:

Hab uebrigens noch ne Klasse

Du musst schon den ganzen Code herzeigen wenn wir darin Leaks finden sollen.

Mechanics

@manni66 sagte in Wo leake ich hier memory? Dynamic array:

Beim ersten Aufruf von resize ist *array ein Nullpointer.

Nein, er hat ja einen **.

marcel91200

@manni66 sagte in Wo leake ich hier memory? Dynamic array:

new[] braucht kein sizeof.

worauf ist das jetzt bezogen?

@manni66 sagte in Wo leake ich hier memory? Dynamic array:

Beim ersten Aufruf von resize ist *array ein Nullpointer. Außerdem kopierst du von array.

Auch wenn ich es so aendere, bringt es nichts. double *array = new double[0];
und dann noch memcpy( tmp, *array, (count-1) * sizeof(double) ); Aendert aber auch beides nichts am Fehler

Sorry ich hab nur Klasse, anstatt Datei geschrieben. Da hab ich mich einfach nur verschrieben. Hier nochmal der komplette Code jetzt:

#include "utils.hpp"

#include <iostream>
#include <sstream>
#include <string>


void resize(double **array, int count)
{
    double* tmp = new double[count * sizeof(double)];
    memcpy( tmp, *array, (count-1) * sizeof(double) );

    delete[] *array;
    *array = tmp;
    
}

double *getDynArray(int &size)
{
    double *array = new double[0];
    double tmp;
    int i = 0;

    while (true)
    {
        std::cout << "Enter " << i + 1 << " number:";
        std::cin >> tmp;
        if (std::cin.fail())
        {
            std::cin.clear();
            std::cin.ignore();
            break;
        }
        i++;
        resize(&array, i);
        array[i - 1] = tmp;
    }
    size = i;
    return array;
}

void getIndizes(int *indizes, int size)
{
    for (int i = 0; i < size; i++)
    {
        std::cout << "Enter Indize: " << i + 1 << ":" << std::endl;
        std::cin >> indizes[i];

        while (std::cin.fail())
        {
            std::cout << "invalid input" << std::endl;
            std::cin.clear();
            std::cin.ignore();
            std::cout << "Enter Indize: " << i + 1 << " :" << std::endl;
            std::cin >> indizes[i];
        }
    }
}

int main(int, char **)
{
    int size = 0;
    double *array = getDynArray(size);
    int *indizes = new int[size];
    getIndizes(indizes, size);

    std::cout << isSorted(array, size, indizes);

    delete[] indizes;
    delete[] array;
   
    return 0;
}

Und die andere Cpp Datei:

#include "utils.hpp"
#include <string.h>

bool isPermutation(int *perm, int count)
{   
   const int size = count;
    int *cmp = new int[size];

    for (int i = 0; i < size; i++)
    {
        if ((perm[i] >= size) | (perm[i] < 0))
        {
            delete[] cmp;
            return false;
        }
        cmp[perm[i]] = 1;
    }

    for (int i = 0; i < size; i++)
    {
        if (cmp[i] != 1)
        {   
            delete[] cmp;
            return false;
        }
    }
    delete[] cmp;
    return true;
}

bool isSorted(double *data, int dataCount, int *perm)
{
    for (int i = 0; i < dataCount - 1; i++)
    {
        if (data[perm[i]] > data[perm[i + 1]])
        {
            return false;
        }
    }
    return true;
}
}

@marcel91200 sagte in Wo leake ich hier memory? Dynamic array:

worauf ist das jetzt bezogen?

new double[count * sizeof(double)];

new double[0];

Der resultierende Zeiger darf auch nicht dereferenziert werden.

Und die andere Cpp Datei:

Und du willst mir jetzt erzählen, du hättest ein Programm mit zwei main?

@Mechanics sagte in Wo leake ich hier memory? Dynamic array:

@manni66 sagte in Wo leake ich hier memory? Dynamic array:

Beim ersten Aufruf von resize ist *array ein Nullpointer.

Nein, er hat ja einen **.

Ja - und?

marcel91200

@manni66

@manni66 sagte in Wo leake ich hier memory? Dynamic array:

Der resultierende Zeiger darf auch nicht dereferenziert werden.

wie mache ich das denn sonst?

@manni66 sagte in Wo leake ich hier memory? Dynamic array:

Und du willst mir jetzt erzählen, du hättest ein Programm mit zwei main?

Ich hab lediglich die falsche Datei kopiert. Ist aber jetzt richtig

Schlangenmensch

@marcel91200 sagte in Wo leake ich hier memory? Dynamic array:

wie mache ich das denn sonst?

So:

@SeppJ sagte in Wo leake ich hier memory? Dynamic array:

Wenn du wissen möchtest, wie man korrekt programmiert: Schlag das Stichwort RAII nach. Das ist ein grundlegendes C++-Programmiermuster, bei dem jeder Ressourcenanforderer selber für die Freigabe verantwortlich. Wenn er das korrekt macht (ist nicht so schwer), dann kann auch nix mehr schief gehen.

Nicht dereferenziert werden, darf der Pointer den du von einem newmit size=0zurück bekommst.

@marcel91200 sagte in Wo leake ich hier memory? Dynamic array:

double *array = new double[0];

Wenn RAII jetzt im Moment keine Alternative ist (wobei dir das viele news und deletesparen würde), könntest du resize zum Beispiel die aktuelle Größe mit geben und für 0eine Sonderbehandlung einführen. Oder du könntest direkt eine Mindestgröße allozieren.

wob

@marcel91200 sagte in Wo leake ich hier memory? Dynamic array:

{
    double* tmp = new double[count * sizeof(double)];
    memcpy( tmp, *array, (count-1) * sizeof(double) );

    delete[] *array;
    *array = tmp; 
}

Achtung, hier sind schon 2 Probleme drin.

new nimmt die Anzahl der Elemente als Parameter, nicht die Größe in Bytes (Unterschied zu malloc!) Du solltest also nur new double[count] machen.
Wenn das array noch leer ist am Anfang, dann ist *array == nullptr. Das ist beim ersten Aufruf der Fall. Nun schau mal in die Dokumentation zu memcpy: dort steht

If either dest or src is an invalid or null pointer, the behavior is undefined, even if count is zero.

Du darfst memcpy in diesem Falle also NICHT aufrufen. D.h. du könntest ein if (*array) memcpy... machen - oder, wie ich sehe, das array mit new double[0] initialisieren (was du jetzt ja gemacht hast).

Anderer Punkt:

double *getDynArray(int &size)

Das ist nicht gut. Du hast eigentlich 2 Rückgabewerte, einmal die Größe und einmal den Pointer. Es ist inkonsistent, dass einer dieser Werte als Ref-Argument genommen wird und der andere returnt wird. Es ist sehr verwirrend, weil diese Funktion vom Namen her so aussieht, als würde sie ein dynamisches Array der Größe size erzeugen. Stattdessen ist size aber ein Rückgabewert. Ich würde diese Funktion eher irgendwie in Richtung userInputArray oder wie auch immer nennen, sodass klar ist, dass diese Funktion was anderes tut. Sie könnte im Übrigen einfach ein std::pair<double*, size> returnen, da du diese beiden Werte ja immer zusammen brauchst. Ok, du darfst keine std::-Klassen benutzen? Dann schreibe dir eine eigene Klasse: struct MyDynArray { double *values = nullptr; size_t size = 0; } und returne ein Objekt dieser Klasse - dann kommst du auch schon langsam in Richtung eigene Implementierung von std::vector<double>.

Und bei dem isPermutation:
Wer zur Hölle bringt euch bei, dass man das hier mit dem new und delete so machen soll? Ist dir aufgefallen, dass aus einem einfachen return jetzt ein jeweils delete[] + return geworden ist? Das ist sehr fehleranfällig und sollte daher niemals so gemacht werden. Ein einfacher Fix hierfür wäre, statt int *cmp = new int[size]; einfach auto cmp = std::make_unique<int[]>(size) zu schreiben. Dann können alle deletes weg.
Aber noch was anderes: was macht dein isPermutation? Normalerweise verstehe ich darunder den Vergleich, ob die Elemente eines Arrays eine Permutation der Elemente eines anderen Arrays sind. Du hast aber nur perm*, count als Parameter. Was wird da gemacht? Kommentar zur Funktion fehlt! Und: pointer+größe als Parameter hat in C++ normalerweise nichts verloren. Mir scheint, dein Professor unterrichtet C++, aber verbietet alle Vorteile, die C++ gegenüber C bietet.

Und dein isSorted - warum hat das 3 Argumente? Was prüft das? Da würde ich umgekehrt erwarten, dass es ein Datenarray (inkl. Größe) als Parameter nimmt, aber nicht data und perm.

NACHDEM du isPermutation und isSorted einmal selbst programmiert hast (zu Übungszwecken sicher sinnvoll), solltest du danach aber nur noch std::is_permutation und std::is_sorted nutzen.

Siehe auch:

kein new und delete verwenden! https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r11-avoid-calling-new-and-delete-explicitly
mehrere Rückgabewerte: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-out-multi bzw allgemein https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#f20-for-out-output-values-prefer-return-values-to-output-parameters
zu dem array* als Parameter: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-ap

Schlangenmensch

@wob sagte in Wo leake ich hier memory? Dynamic array:

Wenn das array noch leer ist am Anfang, dann ist *array == nullptr. Das ist beim ersten Aufruf der Fall. Nun schau mal in die Dokumentation zu memcpy: dort steht

Bei ihm gilt *array = nullptr eben für ein leeres Array nicht. new double[0] gibt einen nicht Nullpointer zurück. Behauptet zumindest https://www.cplusplus.com/reference/new/operator new/

wob

@Schlangenmensch sagte in Wo leake ich hier memory? Dynamic array:

@wob sagte in Wo leake ich hier memory? Dynamic array:

Wenn das array noch leer ist am Anfang, dann ist *array == nullptr. Das ist beim ersten Aufruf der Fall. Nun schau mal in die Dokumentation zu memcpy: dort steht

Bei ihm gilt *array = nullptr eben für ein leeres Array nicht. new double[0] gibt einen nicht Nullpointer zurück. Behauptet zumindest https://www.cplusplus.com/reference/new/operator new/

Ich schrieb ja dazu, Zitat: "oder, wie ich sehe, das array mit new double[0] initialisieren (was du jetzt ja gemacht hast)." - das war ursprünglich nicht im Code drin (siehe ersten Post des OP).

Schlangenmensch

@wob ich gehe mir dann mal 'nen Kaffee holen...

marcel91200

Danke fuer die ganzen Beitraege.
Die bool isPermutation(int *perm, int count) Funktion bekommt ein Array uebergeben und ueberprueft ob ausschliesslich jede Zahl von 0 bis count-1 in diesem vorkommt.
bool isSorted(double *data, int dataCount, int *perm) schaut dann ob unsere Permutation (also das Indizes-Array) kombiniert mit dem anderen Eingabe-Array eine sortierte Liste hervorbringt.
Beide Funktionen waren (bis auf den Inhalt) vorgegeben. Ich kann also nichts fuer die Parameter^^

Zu dem new und delete[] vor jedem return:
Ich wollte dieses Array urspruenglich einfach mit int cmp[size] initialisieren, aber in C++ muss dies ja bereits zur Compiletime genau zugeordnet sein, weswegen das einfach nur eine schnelle Moeglichkeit war, um zu testen ob ich weitere Fehlermeldungen bekomme. std::make_unique<int[]>(size) kannte ich aber noch nicht, danke dafuer.

Mein Code funktioniert jetzt auf dem Testserver, auch wenn ich immer noch eine Warning bekomme und memory geleakt wird. Danke fuer die Hilfe
HEAP SUMMARY:
in use at exit: 72,704 bytes in 1 blocks
total heap usage: 4 allocs, 3 frees, 72,764 bytes allocated

Jockelx

@wob sagte in Wo leake ich hier memory? Dynamic array:

mehrere Rückgabewerte: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-out-multi

OT: Für mich der schlechteste Tip im gesamten Core-guideline! Also mit tuple als Rückgabewert, struct is super!
Hab ich kurz verwendet und bin noch nicht zum refactoren gekommen und sorgt für dauerhaftes nachfragen: "Was zur Hölle ist tuple<string, string, int>!?!"

wob

@Jockelx sagte in Wo leake ich hier memory? Dynamic array:

@wob sagte in Wo leake ich hier memory? Dynamic array:

mehrere Rückgabewerte: https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-out-multi

OT: Für mich der schlechteste Tip im gesamten Core-guideline! Also mit tuple als Rückgabewert, struct is super!

Da steht doch da genauso: Prefer using a named struct where there are semantics to the returned value. Otherwise, a nameless tuple is useful in generic code.

Der Punkt ist, dass du in generischem Code eben nicht immer sinnvolle Namen hast. Wie bei Pair mit first und second. Das hilft auch nicht.

SeppJ

Mein Code funktioniert jetzt auf dem Testserver, auch wenn ich immer noch eine Warning bekomme und memory geleakt wird.

Es gibt schon einen Grund, warum in der ersten Antwort stand und auch oft wiederholt wurde:

@SeppJ sagte in Wo leake ich hier memory? Dynamic array:

Wenn du wissen möchtest, wie man korrekt programmiert: Schlag das Stichwort RAII nach. Das ist ein grundlegendes C++-Programmiermuster, bei dem jeder Ressourcenanforderer selber für die Freigabe verantwortlich. Wenn er das korrekt macht (ist nicht so schwer), dann kann auch nix mehr schief gehen.

Wenn du das getan hättest und dein Programm nach diesen Maßgaben neu programmiert hättest, dann wärst du längst fertig und hättest zudem was nützliches über C++ gelernt. Mit deinem momentanen Ansatz wirst du nie ein sauberes Ergebnis erhalten und das wenige, was du dabei lernst, ist wie man sich das Leben unnötig schwer macht.

Jockelx

@wob

Stimmt! Sehe ich zum ersten Mal. Jetzt fällt auch noch meine Ausrede "hab ich aus den Coreguidelines übernommen" flach. Danke dafür

wob

@marcel91200 sagte in Wo leake ich hier memory? Dynamic array:

Die bool isPermutation(int *perm, int count) Funktion bekommt ein Array uebergeben und ueberprueft ob ausschliesslich jede Zahl von 0 bis count-1 in diesem vorkommt.

Aha! Dann ist nur der Name der Funktion schlecht, eben weil man an das std::is_permutation denkt, das ja etwas anderes tut. Nenne es doch isPermutationsOfFirstNaturalNumbers - oder denk dir was besseres, kürzeres aus

Jendenfalls hast du da noch einen Fehler drin:

int *cmp = new int[size];

Dieser Code initialisiert die Werte in cmp nicht. Du brauchst aber eine Initialisierung, weil du nachher alle Werte mit 1 vergleichst.
=> nimm stattdessen int *cmp = new int[size]{}; (die {} machen den Unterschied) - und schon werden alle ints mit 0 initialisiert (bool statt in täte es hier natürlich auch). Das std::make_unique<int[]>(size) oder ein std::vector<int>(size) initialisieren automatisch.

bool isSorted(double *data, int dataCount, int *perm) schaut dann ob unsere Permutation (also das Indizes-Array) kombiniert mit dem anderen Eingabe-Array eine sortierte Liste hervorbringt.

Ah! Und schon wieder ist das Naming verbesserungswürdig. Und noch ein genereller Tipp: in C++ wird zur Sortierung standardmäßig normalerweise kleiner-als verwendet. Dein Test mit größer-als geht natürlich auch, aber es ist mehr oder weniger Konvention, solche Tests immer mit dem kleiner-als-Operator durchzuführen.

PS: @SeppJ hat mit seinem Rat recht, allerdings hilft der dir natürlich nur für die reale Welt, aber nicht für deine Übungsaufgaben.

SeppJ

@wob sagte in Wo leake ich hier memory? Dynamic array:

PS: @SeppJ hat mit seinem Rat recht, allerdings hilft der dir natürlich nur für die reale Welt, aber nicht für deine Übungsaufgaben.

Wieso nicht? Vorgabe war, nicht vector, array, etc. zu nutzen. Es hindert ihn doch keiner, die 25 Zeilen für eine eigene vector-Implementierung zu schreiben und damit dann alle Aufgaben trivial in 3 Zeilen zu lösen. Da macht er effektiv das gleiche, was er hier von Hand erfolglos versucht, bloß mit Richtigkeitsgarantie, und lernt zudem noch, wie ein vector funktioniert.

marcel91200

Wie gesagt, die Funktionsnamen wurden vorgegeben, genauso wie die Parameter. Daran sollen wir nichts aendern (dann funktioniert auch der automatische Test nicht mehr), da muesst ihr euch bei meinem Prof (oder wer auch immer sich das alles ausdenkt) beschweren ^^

@SeppJ sagte in Wo leake ich hier memory? Dynamic array:

Wieso nicht? Vorgabe war, nicht vector, array, etc. zu nutzen. Es hindert ihn doch keiner, die 25 Zeilen für eine eigene vector-Implementierung zu schreiben und damit dann alle Aufgaben trivial in 3 Zeilen zu lösen.

Die Aufgabe war, dass wir dies mit einem dynamischen Array loesen sollen und uns somit auch gleich die Speicherzuweisung anschauen, da ist nichts mit "eigene vektor klasse implementieren". Es geht eben darum die einzelnen Konzepte zu verstehen und nicht einfach ne vektor klasse ausm Internet zu kopieren (was sicherlich viele machen wuerden)
RAII schaue ich mir aber an, danke fuer den Tipp.