3. Wo leake ich hier memory? Dynamic array

Wo leake ich hier memory? Dynamic array

  • M

    Hallo,
    ich verstehe leider gerade null wo ich hier genau memory leake, also Speicher alloziere, den ich nicht freigebe. Vielleicht hat ja jemand ne Idee

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

    delete[] *array;
    *array = tmp;   
}
double *getDynArray(int &size)
{
    double *array = NULL;
    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);

    delete[] indizes;
    delete[] array;
   
    return 0;
}
    0
    ?
     1 Antwort
  • Mod

    Wenn's dir um die Lösung deines Problems geht: Verwende kein new/delete in C++, sondern die dafür vorgesehenen Strukturen aus der Standardbibliothek (vector, array, set, etc.). Code wird 10x einfacher als das unintuitive Arrayhandling, und das Problem der Speicherlecks stellt sich gar nicht erst, weil diese Strukturen intern korrekt programmiert sind.

    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. Braucht man halt in der Praxis nie selber zu programmieren, weil 99% der Sachen, die man schon brauchen könnte, schon in der Standardbibliothek sind. Ist aber ganz interessant und lehrreich.

    Wenn du nur wissen möchtest, was an deinem Anforderungs- und Freigabechaos hier konkret falsch ist: Ich habe heute keine Lust auf Puzzlespiele. Wie es wirklich richtig geht, steht im ersten Absatz.

    0
    M
     1 Antwort
  • M

    @SeppJ Dies ist eine Aufgabe fuer die Uni und da sollten wir eben nicht die "vorgesehenen Strukturen aus der Standardbibliothek" nutzen. Trotzdem seh ich selber leider den Fehler gerade nicht, weshalb ich dachte, dass mir jemand anderes hier helfen koennte.
    Aber trotzdem danke fuer die Antwort

    0
  • ?

    @marcel91200 warum nimmst du an, dass du ein leak hast?

    0
    M
     1 Antwort
  • M

    @manni66 Das ist eine Uni Aufgabe die automatisch von nem Server getestet wird und dieser gibt eine Fehlermeldung aus.

    ==24991== HEAP SUMMARY:
==24991==     in use at exit: 72,704 bytes in 1 blocks
==24991==   total heap usage: 25 allocs, 24 frees, 74,996 bytes allocated
==24991== 
==24991== LEAK SUMMARY:
==24991==    definitely lost: 0 bytes in 0 blocks
==24991==    indirectly lost: 0 bytes in 0 blocks
==24991==      possibly lost: 0 bytes in 0 blocks
==24991==    still reachable: 72,704 bytes in 1 blocks
==24991==         suppressed: 0 bytes in 0 blocks
==24991== Reachable blocks (those to which a pointer was found) are not shown.
==24991== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==24991== 
==24991== For lists of detected and suppressed errors, rerun with: -s
==24991== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
==24992== Memcheck, a memory error detector
==24992== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==24992== Using Valgrind-3.17.0 and LibVEX; rerun with -h for copyright info
==24992== Command: ./test_main
==24992== 
PASSED Testing a valid permutation.
==24992== Conditional jump or move depends on uninitialised value(s)
==24992==    at 0x10A1F6: isPermutation(int*, int) (utils.cpp:21)
==24992==    by 0x109653: main (test_main.cc:18)
==24992==  Uninitialised value was created by a heap allocation
==24992==    at 0x48A3E3B: operator new[](unsigned long) (vg_replace_malloc.c:579)
==24992==    by 0x10A11E: isPermutation(int*, int) (utils.cpp:7)
==24992==    by 0x109653: main (test_main.cc:18)
==24992== 
PASSED Testing an invalid permutation.
PASSED Testing valid permutation.
PASSED Checking an unsorted list.
PASSED Checking a sorted list.
PASSED ALL 5 tests.
==24992== 
==24992== HEAP SUMMARY:
==24992==     in use at exit: 72,704 bytes in 1 blocks
==24992==   total heap usage: 4 allocs, 3 frees, 72,764 bytes allocated
==24992== 
==24992== LEAK SUMMARY:
==24992==    definitely lost: 0 bytes in 0 blocks
==24992==    indirectly lost: 0 bytes in 0 blocks
==24992==      possibly lost: 0 bytes in 0 blocks
==24992==    still reachable: 72,704 bytes in 1 blocks
==24992==         suppressed: 0 bytes in 0 blocks
==24992== Reachable blocks (those to which a pointer was found) are not shown.
    0
  • ?

    Ich sehe Nullpointer dereferenzieren und of by one in resize. Vielleicht zerschiesst du da einfach den Heap.

    0
    M
     1 Antwort
  • M

    @manni66
    Du meinst dass ich bei resize ja zu viele bytes kopiere mit memcpy oder? Hab ich tatsaechlich uebersehen, aendert aber nichts am Fehler.

    Hab uebrigens noch ne Klasse, aber da sollte meines Wissens nach kein memory leak auftreten, wenn ich nichts offensichtliches uebersehen habe.

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

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

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

    for (int i = 0; i < count; 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;
}
    0
  • ?

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

    Das ist keine Klasse.

    0
    M
     1 Antwort
  • ?

    new[] braucht kein sizeof.

    0
    M
     1 Antwort

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

    0
  • M

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

    Beim ersten Aufruf von resize ist *array ein Nullpointer.

    Nein, er hat ja einen **.

    0
    ?
     1 Antwort
  • M

    @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;
}
}
    0
    ?
    W
     3 Antworten
  • ?

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

    0
    M
     1 Antwort
  • ?

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

    0
  • M

    @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

    0
  • S

    @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.

    0
  • W

    @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.

    1. 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.

    2. 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:

    0
    S
    J
     2 Antworten
  • S

    @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/

    0
    W
     1 Antwort
  • W

    @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).

    0
  • S

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

    0
Anmelden zum Antworten