Problem mit malloc() und free()

[Andreas (ah3112)]

Hallo,

wie der Titel schon sagt, habe ich ein Problem mit malloc() und free. 
Ich teste gerade auf dem STK600 Programmteile, die ich eigentlich in 
einem größeren Programm verwenden will. Der Controller ist ein 
Atmega2560 mit 32k externem Ram.

Ich will jetzt niemandem zumuten, sich das Programm in Einzelnen 
anzusehen. Daher beschreibe ich das mal kurz.

In der main steht folgendes:

  XMCRA =0;
  XMCRA |= (1<<SRE) |(1<<SRW11)|(1<<SRW10) ;
  XMCRB = 0;

Der Linker wird folgendermassen aufgerufen:

-mrelax -ffunction-sections -Wl,--defsym=__heap_start=0x802200,--defsym=__heap_end=0x807fff

In dem Programm rufe ich eine Funktion auf. In der werden verschiedene 
Arrays und Strukturen über malloc reserviert.

char* recbuf = (char*)malloc(sizeof(RECBUFFER_SIZE));
char* sendbuf = (char*)malloc(sizeof(SENDBUFFER_SIZE));

Am Ende der Funktion wird der Speicher wieder freigegeben.

free(recbuf);
recbuf = NULL;
free(sendbuf);
sendbuf = NULL;

In dieser Funktion wird wird eine weitere Funktion aufgerufen, die 
sendbuf  als Parameter hat und über den Uart den Inhalt von sendbuf 
sendet.
Beim ersten Aufruf der ersten Funktion funktioniert das auch 
einwandfrei. Beim zweiten Aufruf stimmt der Wert, der in sendbuf 
geschrieben wird, in der ersten Funktion noch überein, aber nicht in der 
Funktion, die den Inhalt von sendbuf über den Uart senden soll. Ich habe 
mir an der Stelle, wo sendbuf beschrieben wird, das auch mit printf 
anzeigen lassen. Dort steht es aber noch drin. Aber nicht mehr in der 
Funktion, die den Inhalt senden soll.

Nach dem malloc steht folgendes:

if (!recbuf) {
  printf("Sendbuffer Memory allocation error in messdaten_senden\n");
  return;
}

Dasselbe steht auf für sendbuf. Allerdings scheint das zu funktionieren. 
Denn der Fehler wird mir nicht angezeigt.

RECBUFFER_SIZE und SENDBUFFER_SIZE sind mit #define als 255 im Programm. 
Zum Testen habe ich RECBUFFER_SIZE mal auf 25 gesetzt. Der Fehler blieb. 
Immer beim zweiten Mal, funktioniert es nicht mehr.

Wenn ich die Variablen global anlege und nicht über malloc. Dann kann 
ich die Funktionen ohne Fehler bis zur Unendlichkeit aufrufen. Das läuft 
einwandfrei.

Dasselbe, wenn ich die Variablen lokal in der ersten Funktion anlege.

Beim Versuch das weiter einzugrenzen, habe ich das dann nochmal malloc 
verwendet und versehentlich vergessen die Aufrufe von free am Ende der 
Funktion auszukommentieren. Dabei habe ich festgestellt, dass das dann 
auch einwandfrei läuft. Ich vermute allerdings, da der Speicher immer 
neu reserviert wird, dass dann irgendwann ein Fehler auftaucht, weil 
kein freier Speicher mehr vorhanden ist. Nehme ich free wieder rein, 
habe ich wieder mein Problem.

Ich könnte das Ganze natürlich mit lokalen Variablen machen. Das Problem 
ist aber, dass das später unter einem Multitasking-BS (ucos) laufen soll 
und dann müsste der Stack dementsprechend gross angelegt werden, so dass 
selbst der Atmega2560 an seine Grenzen kommen würde. Daher wäre mir der 
Weg mit malloc lieber.

Den Ram habe ich getestet mit einem Testprogramm. Da wird kein Fehler 
gemeldet. An der Hardware liegt es wohl nicht.

Hat jemand eine Idee oder Tip, woran es liegen könnte?

Ich hoffe, ich konnte das einigermaßen vernünftig erklären.

Vielen Dank und viele Grüße
Andreas

[Udo S. (urschmitt)]

Andreas schrieb:
> In dem Programm rufe ich eine Funktion auf. In der werden verschiedene
> Arrays und Strukturen über malloc reserviert.

Warum willst du einen Puffer, den du die ganze Zeit brauchst über 
dynamisch allozieren? Ein Sende und Empfangspuffer braucht man doch die 
ganze Zeit.

Früher gabs eine goldene Regel für µCs: vermeide dynamische 
Speicherverwaltung wenn irgend möglich.

[Klaus (feelfree)]

Andreas schrieb:
> Hat jemand eine Idee oder Tip, woran es liegen könnte?

In deinem wunderbar dokumentierten Code gibt's zwischen Zeile 230 und 
Zeile 128 eine Race-condition.

[Udo S. (urschmitt)]

Andreas schrieb:
> Aber nicht mehr in der
> Funktion, die den Inhalt senden soll.

Mal ganz blöde gefragt: Du alloziert Speicher für einen Sendepuffer 
rufst die sende Funktion mit einem Zeiger auf den Speicher auf und nach 
dem Aufruf gibst du den Speicher frei?
Kann es sein, dass die sende Funktion asynchron über timer oder uart 
interrupt läuft, und du den Speicher frei gibst bevor alles gesendet 
wurde?

[Harald K. (kirnbichler)]

char* recbuf = (char*)malloc(sizeof(RECBUFFER_SIZE));

Das macht sehr sicher nicht das, was beabsichtigt ist.

Was mag "sizeof (RECBUFFER_SIZE)" liefern? Na?

[Andreas (ah3112)]

Vergesst meinen ganzen Beitrag. Als ich geraden den PC runtergefahren 
hatte und mal kurz mit etwas Abstand drüber nachgedacht habe, kam mir 
ein Gedanke. Der Fehler sass mal wieder vor dem Bildschirm.

Es darf natürlich nicht:

char* recbuf = (char*)malloc(sizeof(RECBUFFER_SIZE));
char* sendbuf = (char*)malloc(sizeof(SENDBUFFER_SIZE));

heißen, sondern

char* recbuf = (char*)malloc(RECBUFFER_SIZE);
char* sendbuf = (char*)malloc(SENDBUFFER_SIZE);

Das passiert, wenn man copy und paste macht. Da sind noch zwei 
Strukturen drin, wo ich die Größe des Speichers mit sizeof reserviere. 
Den Code habe ich genommen und für rec- und sendbuf abgeändert. Aber 
eben nicht vernünftig.

Hätte ich

[c]
char* recbuf = (char*)malloc(sizeof(recbuf));
char* sendbuf = (char*)malloc(sizeof(sendbuf));

geschrieben, hätte es funktioniert.

Udo S. schrieb:
> Kann es sein, dass die sende Funktion asynchron über timer oder uart
> interrupt läuft, und du den Speicher frei gibst bevor alles gesendet
> wurde?
Der Speicher wird erst nach dem Senden freigegeben. Aber danke für den 
Hinweis, das werde ich für andere Sachen im Kopf behalten.

Udo S. schrieb:
> Ein Sende und Empfangspuffer braucht man doch die ganze Zeit.
Für Senden und Empfangen werde ich das auch nicht mehr mit malloc 
machen. Aber die Strukturen, die in den Sendebuffer kopiert werden, 
brauche ich nicht immer. Beim Testen habe ich das jetzt erst einmal mit 
dem Sende- und Empfangsbuffer genauso gehandhabt.

Harald K. schrieb:
> Das macht sehr sicher nicht das, was beabsichtigt ist.
>
> Was mag "sizeof (RECBUFFER_SIZE)" liefern? Na?
Danke, unsere Beiträge haben sich überschnitten. :-)
Aber beeindruckt bin ich, wie schnell Du das gesehen hast. Ich suche 
seit Stunden dran und bin erst nach Erstellung meines Threads darauf 
gekommen.

[Udo S. (urschmitt)]

Harald K. schrieb:
> char* recbuf = (char*)malloc(sizeof(RECBUFFER_SIZE));
>
> Das macht sehr sicher nicht das, was beabsichtigt ist.

+10 für Harald! Gut gesehen.

[Rolf (rolf22)]

Andreas schrieb:
Ich kürze das mal auf das Wesentliche:

func1()
{ char* sendbuf = (char*)malloc(sizeof(SENDBUFFER_SIZE));
  ...
  sendToUART(sendbuf);
  ...
  free(sendbuf);
}

Ist das, was dein langes Elaborat uns sagen soll? Was bedeutet dann 
"Beim zweiten Aufruf stimmt der Wert, der in sendbuf
geschrieben wird, in der ersten Funktion noch überein, aber nicht in der
Funktion, die den Inhalt von sendbuf über den Uart senden soll."? WO 
GENAU stimmt WAS?

Wenn tatsächlich free() einen Fehler verursacht, dann könnte es sein, 
dass sendToUART() oder receiveFromUART() falsch in einen der Puffer 
schreibt, und zwar entweder über dessen Ende hinaus oder (per Interrupt 
oder DMA) nachdem er schon freigegeben ist. Das würde die 
malloc/free-Verwaltung komplett korrumpieren.

[Rolf M. (rmagnus)]

Andreas schrieb:
> Vergesst meinen ganzen Beitrag. Als ich geraden den PC runtergefahren
> hatte und mal kurz mit etwas Abstand drüber nachgedacht habe, kam mir
> ein Gedanke. Der Fehler sass mal wieder vor dem Bildschirm.

So kann's gehen.

> Es darf natürlich nicht:char* recbuf =
> (char*)malloc(sizeof(RECBUFFER_SIZE));
> char* sendbuf = (char*)malloc(sizeof(SENDBUFFER_SIZE));
> heißen, sondernchar* recbuf = (char*)malloc(RECBUFFER_SIZE);
> char* sendbuf = (char*)malloc(SENDBUFFER_SIZE);

Falls du in C programmierst, solltest du auch den Cast weglassen. Der 
ist an der Stelle eher kontraproduktiv. In C++ wäre er allerdings nötig.

> Hätte ich[c]
> char* recbuf = (char*)malloc(sizeof(recbuf));
> char* sendbuf = (char*)malloc(sizeof(sendbuf));
> geschrieben, hätte es funktioniert.

Nö. recvbuf ist ein Zeiger, daher ist sizeof recvbuf auch nur die Größe 
eines Zeigers, was wahrscheinlich das gleiche wie sizeof RECVBUFFER_SIZE 
ist.

[Rolf (rolf22)]

Andreas schrieb:
Klaus schrieb:
> In deinem wunderbar dokumentierten Code gibt's zwischen Zeile 230 und
> Zeile 128 eine Race-condition

ROTFL.

[Michael B. (laberkopp)]

Andreas schrieb:
> das dann nochmal malloc verwendet und versehentlich vergessen die
> Aufrufe von free am Ende der Funktion auszukommentieren. Dabei habe ich
> festgestellt, dass das dann auch einwandfrei läuft

Vermutlich greift er aber nur auf den ersten, nie freigegebenen buffer 
zu.

Lass dir die von malloc gelieferte Adresse ausgeben und die Adresse 
einer lokalen Variablen auf dem Stack und guck, ob die plausibel sind.

[Andreas (ah3112)]

Udo S. schrieb:
> +10 für Harald! Gut gesehen.
Sehe ich auch so. Vor allen Dingen mit welcher Geschwindigkeit.

Michael B. schrieb:
> Lass dir die von malloc gelieferte Adresse ausgeben und die Adresse
> einer lokalen Variablen auf dem Stack und guck, ob die plausibel sind.
Werde ich die nächsten Tag machen. Allerdings läuft es jetzt fehlerlos, 
nachdem ich meinen blöden Fehler behoben habe.

Rolf M. schrieb:
> Nö. recvbuf ist ein Zeiger, daher ist sizeof recvbuf auch nur die Größe
> eines Zeigers, was wahrscheinlich das gleiche wie sizeof RECVBUFFER_SIZE
> ist.
Stimmt. Da war ich in Gedanken schon dabei, dass das ein festes Array 
ist. Allerdings wäre dann auch der malloc nicht nötig.

Rolf M. schrieb:
> Falls du in C programmierst, solltest du auch den Cast weglassen.
Danke. Habe es gerade abgeändert und getestet. Ich dachte, das müsste 
rein.  Da steht jetzt:

char* sendbuf = malloc(SENDBUFFER_SIZE);

Compiler meckert nicht und Programm läuft einwandfrei. Natürlich steht 
das auch bei allen anderen mallocs.

Rolf schrieb:
> Wenn tatsächlich free() einen Fehler verursacht, dann könnte es sein,
> dass sendToUART() oder receiveFromUART() falsch in einen der Puffer
> schreibt, und zwar entweder über dessen Ende hinaus oder (per Interrupt
> oder DMA) nachdem er schon freigegeben ist. Das würde die
> malloc/free-Verwaltung komplett korrumpieren.
Das war es nicht. Free wird auch erst ausgeführt, wenn die Daten 
gesendet werden und vom PC auch die Bestätigung kommt, dass alles 
fehlerfrei empfangen wurde.

Danke nochmal an Alle.