miniparser + malloc + speicherproblem?

[PCMan]

Hallo Forum,
ich weiß mal wieder überhaupt nicht weiter.
Zunächst das Ziel: ein über uart eingehender String der Leerzeichen enthält soll zerlegt werden, dabei sollen die einzelnen befehle in eine art "String Array" abgelegt werden.
Plattform: ATMega32

Hier etwas Code:


typedef char** stringArray;

volatile struct
{
char command[UART_STR_LENGTH]; //255 chars
uint8_t command_ptr;
stringArray args;
size_t count;
} cmd;

stringArray MallocStringArray(size_t SizeOfOneString, size_t StringCount)
{
char** t=(char **)malloc(StringCount*sizeof(char*));
size_t i;
for(i=0;i<StringCount;++i)
t[i]= (char*) malloc(SizeOfOneString);
return t;
}

void FreeStringArray(stringArray StringArray, size_t StringCount)
{
size_t i;
for(i=0;i<StringCount;++i)
free(StringArray[i]);
free(StringArray);
}

uint8_t parse()
{
size_t i = 0, j = 0, alen = 0;
char *token, *tmp;
char *cmd_backup = (char *) malloc( strlen((char *)cmd.command) * sizeof(char)+1 );

strcpy(cmd_backup, (char *)cmd.command); //kopie anlegen
cmd.count = 0;

//zähle anzahl wörter:
token = strtok_r(cmd_backup," ",&tmp);
if (token == NULL)
{
uart_puts_P("Error: unable to parse format.\n\r#");
return PARSE_ERROR;
}

alen = strlen(token); //länge des ersten tokens
while((token = strtok_r(NULL," ",&tmp)) !=NULL)
{
if(strlen(token) > alen) alen = strlen(token); //ist dieses token länger? Wenn ja, dann neue länge zuweisen...
i++; //mit jedem token gibts auch einen befehl mehr
}

cmd.count = i; // = Anzahl befehle

cmd.args = MallocStringArray(alen,i);


//da cmd_backup zerhackt wurde, neue Kopie anlegen.
strcpy(cmd_backup, (char *)cmd.command);

//Befehle aus kette Lösen und ins "StringArray"
//cmd.args[0] = strtok_r(cmd_backup," ",&tmp);
strcpy(cmd.args[0], strtok_r(cmd_backup," ",&tmp));
for (j = 1; j<=i; j++)
{
//cmd.args[j] = strtok_r(NULL," ", &tmp);
strcpy(cmd.args[j], strtok_r(NULL," ",&tmp));
}


free(cmd_backup); //allozierten Speicher freigeben.

//Ausgabe der Befehle
for (uint8_t k = 0; k<=cmd.count; k++)
{
uart_puts("\n\rdebug info:");
uart_puts(cmd.args[k]);
}

return PARSE_ERROR; //<- steht hier nur zu debugzwecken!
return PARSE_OK;
}


Die folgende Routine wird von main regelmäßig aufgerufen:



void rs232_receive_and_execute()
{
uint8_t c = uart_getc();
if (!(c & UART_NO_DATA) && !( c & UART_FRAME_ERROR ) && !( c & UART_OVERRUN_ERROR ) && !( c & UART_BUFFER_OVERFLOW )&& c != 0)
{
//sicherheitsvorkehrungen fehlen noch...
//13 == ENTER
//127 == Backspace
if (c == 13) //es wurde Befehl abgeschickt...
{
uart_putc(c);
cmd.command[cmd.command_ptr]='\0';
cmd.command_ptr = 0;
uart_puts_p(p_nr);
if (strlen((char*)cmd.command) == 0) //wenn also nur ENTER gedrückt wurde...
{
uart_puts_p(p_prompt);
return;
}


if (parse()==PARSE_ERROR)
{
FreeStringArray(cmd.args,cmd.count);
return;
}

... unkritischer code ...

FreeStringArray(cmd.args,cmd.count);


... wenn nicht character 13 empfangen wurde wird das empfangene Zeichen einfach nur an cmd.command angehängt.
}


Nun zu Debugzwecken beende ich die Funktion parse immer mit PARSE_ERROR, sodass auch immer FreeStringArray aufgerufen wird. Ich glaube also nicht, dass ich hier einen MemoryLeak habe.

Das Problem ist, dass wenn ich über uart einen befehl sende, zB "abc def ghi jkl" funktioniert das beim ersten aufruf normal, also es werden korrekt alle eingaben wieder zurückgesendet. Aber wenn ich dann eine Eingabe der Form "hallo blubb test muhkuh malloc error" eingebe stürzt der AVR ab oder ich bekomme nur noch Schmodder zurück.

Habt Ihr eine Idee, wo das Problem liegen könnte? Ich benutze malloc sehr selten, aber diesmal dachte ich es wäre sinnvoll es zu verwenden.

Bei einer Zuweisung der Art
cmd.args[j] = strtok_r(NULL," ", &tmp);
ist komischerweise args[0] immer leer.

Irgendwas ist da total faul.

Viele Grüße Simon

[sternst]

1)
Der Speicher für das Stringarray ist zu klein, weil in parse das i um 1 zu klein ist.
Deshalb musst du auch
for (j = 1; j<=i; j++)
und
for (uint8_t k = 0; k<=cmd.count; k++)
schreiben, obwohl in beiden Fällen eigentlich ein "<" statt des "<=" richtig wäre.

2)
char *cmd_backup = (char *) malloc( strlen((char *)cmd.command) * sizeof(char)+1 );
...
return PARSE_ERROR; // MÖGLICHES SPEICHERLECK
...
free(cmd_backup); //allozierten Speicher freigeben.

[PCMan]

Hi,
danke für die schnelle Antwort.
Gegenfrage: wieso habe ich ein Speicherleck, wenn der String kein Leerzeichen enthält?
i habe ich bewusst so gewählt, denn wenn ich "abc abc abc" schicke will ich ein array aus 3 Elementen haben, also der maximale index ist dann 2. Wieso ist das i um 1 zu klein?
VG Simon

[sternst]

Gegenfrage: wieso habe ich ein Speicherleck, wenn der String kein Leerzeichen enthält?
Schau den Post nochmal an. Du warst mit dem Lesen etwas zu schnell ;-)
Ich hatte ihn nochmal editiert und die Stelle im Source markiert.


i habe ich bewusst so gewählt, denn wenn ich "abc abc abc" schicke will ich ein array aus 3 Elementen haben, also der maximale index ist dann 2. Wieso ist das i um 1 zu klein?
Weil i in dem Fall nur 2 ist und du daher nur Speicher für 2 Pointer/Strings holst.

[PCMan]

Achsoooohooo! Stimmt in MallocStringArray hohlt er mir dann einen zu wenig.
Ja klar jetzt versteh ich's auch warum da nix richtig geht. Das mögliche Speicherleck habe ich auch schon gesehen. Allerdings wird diese Stelle im Code doch nie erreicht, weil du mit strtok(x," ",z); immer einen token bekommst, auch wenn du nur einen String ohne Leerzeichen übergibst?
Danke für die Hilfe man,
Viele Grüße Simon

[sternst]

Allerdings wird diese Stelle im Code doch nie erreicht, weil du mit strtok(x," ",z); immer einen token bekommst, auch wenn du nur einen String ohne Leerzeichen übergibst?
Bezüglich des "String ohne Leerzeichen" war ich etwas vorschnell, deshalb hatte ich meinen Post ja auch gleich nochmal editiert.

Aber:
Entweder die Stelle kann erreicht werden, dann hast du ein Speicherleck, oder sie kann garantiert nie erreicht werden, wozu ist der Code dann überhaupt drin? An anderen Stellen, wo eine Überprüfung der Rückgabe auf NULL wirklich zwingend erforderlich wäre, machst du dafür einfach nichts, nämlich bei jedem malloc.

[PCMan]

Hallo,
kannst du das bitte genauer Ausführen?
Meinst du, ich soll schauen ob nach malloc im Rsult NULL drin ist? Ist das der Fall, dann hat die Speicherreservierug fehlgeschlagen und das sollte ich natürlich entsprechend handlen, sonst driftet der µC ins Nirvana ab?
Grüße Simon

[sternst]

Ja.
Wenn malloc den angeforderten Speicher nicht zur Verfügung stellen kann, liefert es NULL zurück.

[PCMan]

Alles klar, hab vielen Dank.
Simon