malloc, realloc und free



  • Hallo,
    Ich ich habe ein kleines Testprogramm zum Thema malloc, realloc und free geschrieben und wollte euch fragen, was Ihr davon haltet. Außerdem stürzt es am Ende immer ab, vielleicht weiß ja Jemand, woran's liegt 🙂

    #include <stdio.h>
    #include <stdlib.h>
    
    int main()
    {
    char **Array = (char**)malloc(0);
    for(int i = 1; i < 1000; ++i)
      {
      (char**)realloc(Array, sizeof(char*) * i);
      Array[i] = "ABC";
      if(Array == NULL)
        {
        free(Array);
        return -1;
        }
      wprintf(L"%i\n", i);
      }
    free(Array);
    wprintf(L"---FINISH---\n");
    return 0;
    }
    

    Danke im Voraus für Eure Hilfe
    MfG
    DragonRaider

    Ps.:
    Es gibt noch

    ---FINISH---
    

    aus und dann stürzt es ab 😕


  • Mod

    DragonRaider schrieb:

    wollte euch fragen, was Ihr davon haltet.

    Außerdem stürzt es am Ende immer ab

    Was soll man davon noch halten? Das ist ganz schön schlecht, findest du das nicht selber auch?

    Dein Programm macht sehr vieles falsch und das, was es richtig macht, macht es komisch. Ich weiß nicht, ob du das was du tust wirklich so beabsichtigt hast.

    Unvollständige Liste der Fehler und komischen Stellen:
    -wprintf braucht wchar.h
    -Erklär mal die Funktion des (char**) in char **Array = (char**)malloc(0);
    -Erklär mal, was du von malloc(0) erwartest.
    -Erklär mal das (char**) in (char**)realloc(Array, sizeof(char*) * i);

    • Array[i] = "ABC"; : Was meinst du, passiert hier?
    • Array[i] = "ABC"; : Und wenn Array ein Nullzeiger ist?
    • Array[i] = "ABC"; : Wie groß ist i? Wie groß ist das Array, auf das Array zeigt?
      -Zeile 11: Was soll das werden?
      -Zeile 13: Du free'st also den Nullzeiger?
      -(w)printf für Zeichenkettenliterale?
      Und das Kernstück:
      -realloc: Völlig falsch benutzt. Guck dir die Funktion noch einmal an.


  • Hi SeppJ,
    Danke erstmal, für diese durchaus Kritische Antwort 🙂
    -Ja... Aber, wenn dem so wäre, warum kann ich dann sogar eine wchar_t deklarieren?
    -Mein Kompiler schlägt bei nem nicht implizit ausgewiesenem(falls man das so sagen kann, mir fällt gerade das Wort nicht ein) Cast an, deiner nicht? Z.B. von void* zu char**...
    -Wenn ich das weg lasse, stürzt das Programm sofort ab(hatte ich ja anfangs)...
    -Siehe Punkt 2(hatte da vorher ein Array = ...)
    -Dass er den Pointer an der Stelle Array[i] "auf "ABC" setzt"(falscher Ausdruck, ich weiß)
    -Tja, dass ist dann schlecht 😉 wird gefixt
    -Groß genug xD
    -Hätte in Zeile 10 gemusst, danke
    -Nicht drüber nachgedacht, sry
    -Ich sehe da nur nen numerischen Literal, aber wo ein Zeichenkettenliteral?
    -Wird gemacht

    Ich kompiliere mit MinGW(g++) unter Windows 8.

    SeppJ schrieb:

    DragonRaider schrieb:

    wollte euch fragen, was Ihr davon haltet.

    Außerdem stürzt es am Ende immer ab

    Was soll man davon noch halten? Das ist ganz schön schlecht, findest du das nicht selber auch?

    Hattest du heute nen schlechten Tag?

    MfG
    DragonRaider



  • g++ ist ein C++ Compiler C++ != C (dehalb auch das void* gemecker)
    malloc(0) gibt hier wohl NULL zurück, dokumentation von realloc lesen
    wiso wird der Rückgabewert von realloc keiner Variable zugewiesen,
    warum benutzt du nach realloc Array, sie könnte auf nicht reservierten Speicher zeigen


  • Mod

    DragonRaider schrieb:

    Hi SeppJ,
    Danke erstmal, für diese durchaus Kritische Antwort 🙂
    -Ja... Aber, wenn dem so wäre, warum kann ich dann sogar eine wchar_t deklarieren?

    Weil das zufällig irgendwo indirekt gefunden wird. Es ist aber falsch, sich auf solche Indirektionen in der Standardbibliothek zu verlassen, da sie sich ganz schnell ändern können. Bei meinem Compiler (GCC 4.8.0) ist beispielsweise wprintf nicht bekannt, wenn man nur stdio.h und stdlib.h einbindet.

    -Mein Kompiler schlägt bei nem nicht implizit ausgewiesenem(falls man das so sagen kann, mir fällt gerade das Wort nicht ein) Cast an, deiner nicht? Z.B. von void* zu char**...

    Dann hast du einen C++-Compiler für C genutzt. Tu das nicht.

    -Wenn ich das weg lasse, stürzt das Programm sofort ab(hatte ich ja anfangs)...

    Dann hast du wohl ein realloc auf einen uninitialisierten Zeiger angewandt. Initialisier doch mit NULL. NULL gibt bei realloc ein genau definiertes Verhalten. malloc(0) ist hingegen "implementation defined"

    -Siehe Punkt 2(hatte da vorher ein Array = ...)

    Gleiche Erwiderung wie oben.

    -Dass er den Pointer an der Stelle Array[i] "auf "ABC" setzt"(falscher Ausdruck, ich weiß)

    Richtig. Dann hast du wenigstens nicht geglaubt, dass das irgendwie die Zeichenkette kopiert wird, was ich befürchtet hatte.

    -Tja, dass ist dann schlecht 😉 wird gefixt

    Gut so.

    -Groß genug xD

    Sicher?

    -Hätte in Zeile 10 gemusst, danke

    Jain. Nein, weil du realloc ohnehin falsch benutzt. Ab ja, wenn du es falsch benutzt, dann ist es auch eine gute Idee, den Rückgabewert zu prüfen.

    -Ich sehe da nur nen numerischen Literal, aber wo ein Zeichenkettenliteral?

    wprintf(L"---FINISH---\n");
    

    Hattest du heute nen schlechten Tag?

    Nein, abgesehen davon, dass es zu heiß ist. Aber welche Meinung erwartest du zu einem Programm, das nicht funktioniert, außer, dass es falsch ist?



  • Hi SeppJ,
    Danke für deine Antworten. Ich habe sehr viel dazu gelernt(würde ich behaupten). Danke dafür. Meine Code sieht jetzt so aus:

    #include <stdio.h>
    #include <stdlib.h>
    #include <wchar.h>
    
    int main()
    {
    char **Array = NULL;
    for(int i = 1; i < 1000; ++i)
      {
      Array = (char**)realloc(Array, sizeof(char*) * i);
      if(Array == NULL)
        {
        return -1;
        }
    
      Array[i] = (char*)calloc(sizeof(char), 3);
      if(Array[i] == NULL)
        {
        free(Array);
        return -1;
        }
    
      Array[i] = "ABC";
    
      wprintf(L"%i\n", i);
      }
    free(Array);
    wprintf(L"---FINISH---\n");
    return 0;
    }
    

    Wahrscheinlich immer noch nicht perfekt, aber ich habe mir Mühe gegeben 🙂 Ich nutze immer noch den C++ Compiler, da ich malloc, realloc und calloc in einem C++ Programm nutzen möchte, sonst hätte ich den C Compiler gewählt.
    Leider stürzt das Programm jetzt bei 2-4 ab(bzw. es gibt -1 zurück). Darum auch die Aussage oben 😉
    MfG
    DragonRaider

    Edit:
    Ich hatte die genaue Bedeutung des Wortes "Zeichnekettenliteral" noch nicht gekannt, deshalb habe ich bei Wikipedia nach geguckt. Da steht ua., dass sie zu Laufzeit festgelegt werden können, nicht müssen. An diesem Punkt hatte ich mich verlesen. Mein Fehler 🙂


  • Mod

    So richtig korrekt wird realloc benutzt, indem du erst einmal die neue Adresse temporär speicherst. Denn falls du eine Null zurück bekommst, so hat realloc das alte Array nicht freigegeben!

    Wie ich schon 2x sagte, schreibst du hinter die Grenzen deines Arrays.

    Du allokierst nun in Zeile 16 Speicher und schmeißt in Zeile 23 den Pointer darauf weg. Speicherloch!

    sizeof(char) ist per Definition immer 1.

    Ich nutze immer noch den C++ Compiler, da ich malloc, realloc und calloc in einem C++ Programm nutzen möchte,

    Stopp! In C++ benutzt du niemals Code der auch nur annähernd so aussieht wie dieser und auch ganz bestimmt kein malloc, realloc oder calloc. Siehe in meiner Signatur Listen von guten C++-Büchern. C++ geht ganz anders.



  • SeppJ schrieb:

    Du allokierst nun in Zeile 16 Speicher und schmeißt in Zeile 23 den Pointer darauf weg. Speicherloch!

    sizeof(char) ist per Definition immer 1.

    Und selbst wenn du meinst das Array[i] = "ABC"; kopiert die Zeichen ABC in dein vorher mit calloc besorgten Speicher, reicht der Platz nicht aus. (Stichwort: Nullterminierung)

    Und ein free(Array); am Ende (oder zwischendrin) reicht nicht aus.
    Du musst schon noch jedes deiner Array[i] einzeln freigeben.



  • Hi,
    Ich dachte mir mal so, ich versuch's noch mal 🙂

    #include <stdlib.h>
    #include <iostream>
    
    int main() 
    { 
    char **Array = NULL; 
    char **tmp;
    for(int i = 1; i < 1000; ++i) 
      { 
      tmp = realloc(Array, sizeof(char*) * i); 
      if(tmp == NULL) 
        { 
        int j;
        for(j = 0; j < i; ++j)
          {
          free(Array[j]);
          } 
        free(Array);
    
        return -1; 
        } 
      Array = tmp;
    
      Array[i - 1] = calloc(sizeof(char*) * 3);
      if(Array[i - 1] == NULL)
        {
        int j;
        for(j = 0; j < i - 1; ++j)
          {
          free(Array[j]);
          } 
        free(Array);
    
        return -1; 
        }
    
      Array[i - 1] = "ABC"; 
      std::cout << i << std::endl; 
      } 
    
    int i;
    for(i = 0; i < 999; ++i)
      {
      free(Array[i]);
      } 
    free(Array); 
    
    std::cout << "---PROGRAM FINISH---" << std::endl;
    return 0; 
    }
    


  • 1. das Programm wird nicht kompileren es ist weder gültiges C noch C++
    2. du solltest das freigeben in eine eigene funktion auslagern, denn der Code ist sehr unübersichtlich
    3. statt

    tmp = realloc(Array, sizeof(char*) * i);
    

    empfehle ich

    tmp = realloc(Array, sizeof(*tmp) * i);
    

    4. in der Zeile danach rüfst du free mit dem nicht reservierten Array[i-1] auf
    5. -1 wird wahrscheinlich nicht als Rückgabewert ankommen, der ist meist ein uint8_t, du könntest die makros EXIT_SUCCESS EXIT_FAILURE aus stdlib.h nutzen
    6. mit = werden keine Strings kopiert, sondern du änderst den Pointer
    7. aber es ist sowiso der reservierte Speicher für "ABC" zu klein, denn das braucht 4 chars
    8. du rufst wegen 6. free auf statischem speicher auf


Log in to reply