(re)alloc/free - Programm stürzt manchmal ab

Hallo,

Ich habe hier gerade einen kleinen Testcode geschrieben und wollte mal fragen, was an diesem Code in Sachen Speicherverwaltung (realloc/free) falsch ist, denn bei fast jedem Aufruf von free() stürzt das Programm ab.

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
	FILE *f = fopen("test.txt", "r");
	int ch = 0;
	unsigned int cnt = 0;
	char *ln = NULL;

	while(ch != EOF)
	{
		ch = fgetc(f);

		if(ch == '\n' || ch == EOF)
		{
			if(cnt > 0)
			{
				ln[cnt] = '\0';
				cnt = 0;

				printf("line: %s\n", ln);

				/*free(ln);
				ln = NULL;*/ /* hier kommen die Fehler! */
			}
		}
		else
		{
			ln = realloc(ln, ++cnt * sizeof(char));
			if(!ln)
			{
				fprintf(stderr, "malloc");
				exit(1);
			}

			ln[cnt - 1] = ch;
		}
	}
	fclose(f);

	return 0;
}

Vielen Dank schonmal.

rüdiger

Du reservierst keinen Speicher für das '\0' (Z.19). Ansonsten verwendest du realloc auf eine gefährliche weise. Siehe 3.2 http://www.c-plusplus.net/forum/viewtopic.php?t=206606 (btw. sizeof(char) ist immer 1 und kannst du daher weglassen)

(Ich nehme mal an, dass das ganze nur ein einfaches Beispiel sein soll. In Wirklichkeit sollte man nicht für jedes byte neuen Speicher anfordern, sondern lieber in großen Blöcken.)

rüdiger schrieb:

Du reservierst keinen Speicher für das '\0' (Z.19). Ansonsten verwendest du realloc auf eine gefährliche weise. Siehe 3.2 http://www.c-plusplus.net/forum/viewtopic.php?t=206606 (btw. sizeof(char) ist immer 1 und kannst du daher weglassen)

Danke für diese späte Antwort, das war der Fehler.
Ist der Code so korrekt?

#include <stdio.h>
#include <stdlib.h>

int main(void)
{
	FILE *f = fopen("test.script", "r");
	int ch = 0;
	unsigned int cnt = 0;
	char *ln = NULL;

	while(ch != EOF)
	{
		ch = fgetc(f);

		if(ch == '\n' || ch == EOF)
		{
			if(cnt > 0)
			{
				void *tmp = realloc(ln, cnt + 1);
				if(!tmp)
				{
					perror("malloc");
					exit(1);
				}
				ln = tmp;

				ln[cnt] = '\0';
				cnt = 0;

				printf("%s\n", ln);

				/*free(line);
				line = NULL;*/
			}
		}
		else
		{
			void *tmp = realloc(ln, ++cnt);
			if(!tmp)
			{
				perror("malloc");
				exit(1);
			}
			ln = tmp;

			ln[cnt - 1] = ch;
		}
	}
	fclose(f);

	/*free(line);
	line = NULL;*/

	return 0;
}

Eine Frage hätte ich da noch:
Reicht es, wenn ich das free() am Ende (nach dem fclose()) aufrufe oder sollte ich es besser jedes mal nach dem printf() aufrufen?

rüdiger schrieb:

(Ich nehme mal an, dass das ganze nur ein einfaches Beispiel sein soll. In Wirklichkeit sollte man nicht für jedes byte neuen Speicher anfordern, sondern lieber in großen Blöcken.)

Ja, es ist natürlich nur ein Beispiel.

pointercrash**

Gast74583 schrieb:

Ist der Code so korrekt?

Eine Frage hätte ich da noch:
Reicht es, wenn ich das free() am Ende (nach dem fclose()) aufrufe oder sollte ich es besser jedes mal nach dem printf() aufrufen?

rüdiger schrieb:

(Ich nehme mal an, dass das ganze nur ein einfaches Beispiel sein soll. In Wirklichkeit sollte man nicht für jedes byte neuen Speicher anfordern, sondern lieber in großen Blöcken.)

Ja, es ist natürlich nur ein Beispiel.

Das sind so Fragen wie: soll ich nach jeder Tasse Kaffee die Espressomaschine putzen?
Kannst Du gerne machen, wenn Du Zeit dafür hast, ist aber im Prinzip sinnlos. Du verbrätst mit zeichenweisem realloc() sowieso schon enorm Rechenzeit. Als einzig sinnvolles Argument wäre noch möglicherweise anzuführen, daß man das free optisch nicht zu weit vom malloc entfernt haben will, was bei diesem simplen Bsp. nicht wirklich stichhaltig ist.

Einen weiteren groben Schnitzer sehe ich jetzt nicht.

Danke für die Antwort.
Um Performance ging es mir bei diesem Beispiel auch nicht.

rüdiger

Wenn man: realloc fehlschlägt, dann gibt es nicht automatisch ln frei! Du musst also auch nochmal free(ln) in den Fehlerfällen aufrufen.

pointercrash**

rüdiger schrieb:

Wenn man: realloc fehlschlägt, dann gibt es nicht automatisch ln frei! Du musst also auch nochmal free(ln) in den Fehlerfällen aufrufen.

Also jetzt bist Du unnötig pingelig - er killt ja sein Programm mit exit, wenn realloc fehlschlägt.
Ist ja wie einen Gehenkten vor der Beerdigung zur Sicherheit nochma aufm elektrischen Stuhl zu grillen.

rüdiger

pointercrash() schrieb:

rüdiger schrieb:

Wenn man: realloc fehlschlägt, dann gibt es nicht automatisch ln frei! Du musst also auch nochmal free(ln) in den Fehlerfällen aufrufen.

Also jetzt bist Du unnötig pingelig - er killt ja sein Programm mit exit, wenn realloc fehlschlägt.
Ist ja wie einen Gehenkten vor der Beerdigung zur Sicherheit nochma aufm elektrischen Stuhl zu grillen.

:p
Das ist ja nur n kurzes Beispiel, weil er den Umgang mit realloc/free lernen will. Dann soll er es auch gleich richtig machen.