"free" führt zu Programmabstürzen

[SeppJ]

pferdefreund schrieb:

if (pointer != NULL) free(pointer);

Was willst du denn dadurch erreichen?

[Wutz]

pferdefreund schrieb:

if (pointer != NULL) free(pointer);

Nonsens pur.
Verdeckt Designfehler und lässt auch sonst auf mangelnde Übersicht über seinen eigenen Code schließen.

[SeppJ]

Wutz schrieb:

Verdeckt Designfehler

Es macht doch einfach nur gar nichts.

Es lässt natürlich auf mangelndes Verständnis von free und dem Sinn von Nullzeigern schließen. Aber das wollte ich durch meine Gegenfrage nochmal klären.

[Wutz]

if (pointer != NULL) free(pointer);

Zeigt deswegen eigene Inkompetenz an, da hier ganz offensichtlich eine mehrfache Freigabe desselben Speicherbereiches versucht wird zu verhindern, was wiederum auf eine Vielzahl und/oder falsche Platzierung von free schließen lässt, was wiederum auf mangelnden Überblick schließen lässt, was wiederum auf den Einsatz des laienhaften und fehlerverdeckenden

free(p); p=NULL;

schließen lässt, was auch nur wieder fehlerverdeckend wirkt.

[drmemory]

also ich hab jetzt meine exe mit dem program "drmemory" inspiziert.

dabei sind 4 mehr oder weniger schwere memory-fehler (drmemory: "32 byte(s) of leak(s)" !!!).

ich hoff, ich krieg die alleine wieder raus.

noch eine frage: was mit adressangaben ala:
"Error #2: UNADDRESSABLE ACCESS: reading 0x00341070-0x00341074 4 byte(s)"

machen? ja klar, hier ist wahrscheinlich das offset zu den opcodes angegeben, aber ich brauch ja die stelle im code... und ich will jetzt nicht mit irgendeinem disassembler hantieren, nur um irgendwelche speicherlecks zu finden

[seldon]

Wutz schrieb:

if (pointer != NULL) free(pointer);

Zeigt deswegen eigene Inkompetenz an, da hier ganz offensichtlich eine mehrfache Freigabe desselben Speicherbereiches versucht wird zu verhindern, was wiederum auf eine Vielzahl und/oder falsche Platzierung von free schließen lässt, was wiederum auf mangelnden Überblick schließen lässt, was wiederum auf den Einsatz des laienhaften und fehlerverdeckenden

free(p); p=NULL;

schließen lässt, was auch nur wieder fehlerverdeckend wirkt.

Du bist mir ein Scherzkeks, aus zwei Zeilen Code Dinge über die Architektur ableiten zu wollen. Das Unschöne an

if (pointer != NULL) free(pointer);

ist, dass es redundant ist.

free(pointer);

macht genau das selbe, weil

free(NULL);

nichts macht (s. ISO/IEC 9899:1999 7.20.3.2 (2)). Fehler werden dadurch nicht verdeckt, und wenn du dich darauf verlassen hast, dass free(NULL); sich beschwert...nun ja, ich wäre an deiner Stelle vorsichtig damit, Begriffe wie "Inkompetenz" oder "laienhaft" herumzuwerfen.

Ferner ist das setzen von Zeigern auf NULL nach Freigabe nicht als solches verwerflich. Habe ich beispielsweise einen verkettete Liste und lösche das letzte Element, so ist es durchaus legitim, wenn tail->next danach NULL ist. Ähnliches gilt für Blattknoten in Bäumen, und eigentlich für so ziemlich alle verlinkten Datenstrukturen, für die man malloc und free üblicherweise so braucht. Der Hauptanwendungszweck dieser Dinge ist ja nicht, stapelbasierte Speicherverwaltung in umständlich nachzubauen.

@drmemory: Ich denke, du willst http://drmemory.org/docs/ mal genauer durchsehen, da steht zu deinen Fragen einiges. Insbesondere http://drmemory.org/docs/page_prep.html .

[Invalid Heap Argument]

ich hab ein eigenes struct.

ich alloziere speicherplatz:

MYSTRUCT *test=(MYSTRUCT *)malloc(sizeof(MYSTRUCT));
if( test == NULL)
{
    return 1;
}

free(test); // <-- "Invalid Heap Argument"

wie kann ich das invalide argument zu einem validen argument machen?
und ich verstehe nich wie vor nicht, was genau jetzt da das problem ist...
das speicher wird reserviert, kann aber nicht geschlossen werden.

[SeppJ]

MFK schrieb:

Die Ursache ist mit an Sicherheit grenzender Wahrscheinlichkeit ein Bug in deinem Code. Überschreiben von Array-Grenzen ist der häufigste Fehler.

It0101 schrieb:

Erstmal wäre es sinnvoll, wenn du deinen Quellcode postest, damit dir überhaupt jemand helfen kann.

Edit: aber keine 1000 Zeilen, sondern ein Minimalbeispiel bei dem der Fehler noch auftritt.

DirkB schrieb:

ratlos! schrieb:

(ich hab grad keine zeit die genauen fehler rauszusuchen.

Das System macht die Meldungen nicht zum Spaß. Und wie das bei Computern so üblich ist, ist die genaue Meldung wichtig.

ratlos! schrieb:

(auch der compiler meldet beim compilieren keine fehler.

Auch keine Warnungen? Die musst du auch beachten. Hast du -Wall gesetzt?

ratlos! schrieb:

(ich bin ratlos

Das wirst du auch bleiben, wenn du nicht mehr Informationen raus gibst.

SeppJ schrieb:

da der Fehler sich höchstwahrscheinlich an ganz anderer Stelle äußert, als da wo die eigentliche Ursache liegt.

Das alles gilt immer noch unverändert. Du hast nichts zur Verbesserung der Lage beitragen können. Dir kann man so nicht helfen.

Der Codefetzen den du zeigst hat mindestens schon einmal einen fehlerhaften, weil unnötigen, Cast drin, der eventuelle Fehler bei der Einbindung von malloc verdeckt. Entferne ihn! Das ist zwar höchstwahrscheinlich nicht der Fehler, aber schließt zumindest eine kleine Möglichkeit aus.

[nachtrag]

p.s.:
ich hab einige zeilen code eingebaut:

MYSTRUCT *test=(MYSTRUCT *)malloc(sizeof(MYSTRUCT));
if( test == NULL)
{
    return 1;
}

free(test); // <-- "Invalid Heap Argument"

if( test != NULL)
{
    printf("ERROR\n");
}

und so wie es aussieht, wird der speicher tatsächlich nicht geschlossen (es wird "ERROR" ausgegeben).
was kann ich da machen?

[SeppJ]

nachtrag schrieb:

und so wie es aussieht, wird der speicher tatsächlich nicht geschlossen (es wird "ERROR" ausgegeben).
was kann ich da machen?

Wieso sollte da nicht "ERROR" ausgegeben werden? free verändert schließlich sein Argument nicht^*.

^*: Und das ist auch gut so! Siehe meinen, Wutz und seldons Rant über NUllpointermissbrauch.

[ooooook]

SeppJ schrieb:

Der Codefetzen den du zeigst hat mindestens schon einmal einen fehlerhaften, weil unnötigen, Cast drin, der eventuelle Fehler bei der Einbindung von malloc verdeckt. Entferne ihn! Das ist zwar höchstwahrscheinlich nicht der Fehler, aber schließt zumindest eine kleine Möglichkeit aus.

ok. sollte man generell nicht einen mit malloc reservierten speicher umcasten?

[SeppJ]

ooooook schrieb:

ok. sollte man generell nicht einen mit malloc reservierten speicher umcasten?

Man sollte in C generell keinen von malloc und Konsorten zurückgegebenen Pointer umcasten.

Das gilt natürlich auch für sämtliche andere unnötige Pointercasts. In C sind void* implizit in andere (Daten-)Zeigertypen konvertierbar. Wenn man nun jedoch einen unnötigen Cast einbaut, dann kann der Compiler keine Fehler in der Programmlogik mehr entdecken, zum Beispiel wenn man einen int einem Pointer zuweist^*. Die Casts in C sind in ihrem Funktionsumfang sehr mächtig (in C++-Sprech ist ein C-Cast eine Kombination von static-, const- und reinterpret-Cast), man sollte sie sparsam einsetzen und sich ganz genau bewusst sein, was sie tun.

^*: Das wäre hier beispielsweise der Fall, wenn man stdlib.h nicht eingebunden hat. Die Funktion malloc wird dann implizit als int malloc(Argumenttyp); deklariert. Falls sizeof(int) != sizeof(void*) (z.B. 64-Bit System), dann kracht es zur Laufzeit. Das ist auch kein rein theoretischer Fehler, das kam hier im Forum schon öfters vor.

P.S.: Man sollte erwähnen, dass es durchaus auch Leute gibt, die das anders sehen. Aber deren Argumente haben mir nie eingeleuchtet. Die gefühlte Mehrheit (insbesondere der "Guru"-Programmierer) scheint gegen den Cast zu sein.

[--lösung--]

ich habe jetzt etwas rumprobiert und herausgefunden:
man sollte an speicher, der schon reserviert wurde, nicht an anderen reservierten speicher hängen.

z.b.

char **pointer=malloc(sizeof(char *);

char pointer[0]=malloc(500);

free(pointer[0]);
free(pointer);

anscheinend löst sowas speicherlecks aus.

[SeppJ]

--lösung-- schrieb:

ich habe jetzt etwas rumprobiert und herausgefunden:
man sollte an speicher, der schon reserviert wurde, nicht an anderen reservierten speicher hängen.

z.b.

char **pointer=malloc(sizeof(char *);

char pointer[0]=malloc(500);

free(pointer[0]);
free(pointer);

anscheinend löst sowas speicherlecks aus.

Unsinn. Wenn man den Code mal so weit korrigiert, dass er überhaupt compilierbar ist:

char **pointer=malloc(sizeof(char *));

  pointer[0]=malloc(500);

  free(pointer[0]);
  free(pointer);

Dann ist das 1a korrektes, fehlerloses Verhalten.

Es gilt immer noch:

MFK schrieb:

Die Ursache ist mit an Sicherheit grenzender Wahrscheinlichkeit ein Bug in deinem Code. Überschreiben von Array-Grenzen ist der häufigste Fehler.

It0101 schrieb:

Erstmal wäre es sinnvoll, wenn du deinen Quellcode postest, damit dir überhaupt jemand helfen kann.

Edit: aber keine 1000 Zeilen, sondern ein Minimalbeispiel bei dem der Fehler noch auftritt.

DirkB schrieb:

ratlos! schrieb:

(ich hab grad keine zeit die genauen fehler rauszusuchen.

Das System macht die Meldungen nicht zum Spaß. Und wie das bei Computern so üblich ist, ist die genaue Meldung wichtig.

ratlos! schrieb:

(auch der compiler meldet beim compilieren keine fehler.

Auch keine Warnungen? Die musst du auch beachten. Hast du -Wall gesetzt?

ratlos! schrieb:

(ich bin ratlos

Das wirst du auch bleiben, wenn du nicht mehr Informationen raus gibst.

SeppJ schrieb:

da der Fehler sich höchstwahrscheinlich an ganz anderer Stelle äußert, als da wo die eigentliche Ursache liegt.

[pferdefreund]

Bitte genau lesen - bei diesem konstrukt das printf, wo man ist und prüfen, warum das Teil NULL ist.
Wenn ich mal so Probleme habe,
vor dem free
if (pointer == NULL) {
printf("Hier bei -- sollte nicht sein\n");
}
else
free (pointer);

Das war als Debugging-Hilfe gedacht und nicht als ordentliches Programmdesign.

[SeppJ]

Aber wieso sollte der Zeiger jemals Null werden? Hier wurde doch der Heap zerschossen oder ein falscher Pointer ge-free-ed, Nullzeiger kommen nirgends vor.