[gelöst]realloc + Segementation fault

[Fabi++ 0]

Hi,

ich versuche grade eine Text-Datei auszulesen und den Inhalt zweilenweise in einer Liste aus char-Arrays abzuspeichern.
Wenn ich platz für die dritte Zeile reservieren will mit realloc() erhalte ich einen segementation fault (Funktion appendLineToList).
Ich finde beim besten Willen keine Erklärung für den Fehler.

Hier mal der Source-Code:

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

/*Boolsche Werte*/
#define MTRUE 1
#define MFALSE 0
/*Ende der Zeile*/
#define EOL 10 // Line Feed

int printLineList(char **pList, int *pAnzLines)
{
	/*Variablen*/
	int i;

	printFunc(__func__);

	fprintf(stdout, "\n\tAnzahl Zeilen: %d in %s\n", *pAnzLines, __func__);

	/*Ausgabe*/
	fprintf(stdout, "Inhalt der Liste: \n");
	for(i = 0; i < *pAnzLines; i++)
	{
		fprintf(stdout,"\t%d %s\n", i, pList[i]);
	}
}

void appendLineToList(char ***pList, char* pLine, int *pAnzLines)
{
	int iNewSize = (*pAnzLines) + 1;

	/*Debug-Informationen*/
	printFunc(__func__);

	/*Speicher für weiteren Eintrag in der Liste reservieren*/
	*pList = realloc( *pList, sizeof( char* ) * iNewSize + 1);

	if(*pList != NULL)
	{
		*pList[(*pAnzLines)] = NULL;
		*pList[(*pAnzLines)] = malloc( sizeof( char ) * strlen(pLine) + 1);

		if(*pList[(*pAnzLines)] != NULL)
		{
			strcpy(*pList[(*pAnzLines)], pLine);
			fprintf(stdout, "Inhalt der Liste %s Position: %d\n", *pList[*pAnzLines], *pAnzLines);
		}
		else
		{
			errDbg(__func__, __LINE__, "Speicher fuer String reservieren (malloc)");
			return EXIT_FAILURE;
		}
	}
	else
	{
			errDbg(__func__, __LINE__, "Speicher fuer weiteren Eintrag in der Liste reservieren (realloc)");
			return EXIT_FAILURE;
	}

}

int readLines(char ***pList, int *pAnzLines, FILE *pFile)
{	
	/*Variablen*/
	char tmpChar[2];
	int iChar;
	char *tmpLine;
	int blnCpy = MFALSE;
	int iCharCnt = 0;
	int blnListInitial = MFALSE;

	printFunc(__func__);

	/*Temporaere Zeile initialisieren*/
	tmpLine = NULL;

	do{
		iChar = fgetc(pFile);
		sprintf(tmpChar, "%c", iChar);

		/*Ende der Zeile abfragen*/
		if(iChar != EOL)
		{
			if(iChar != EOF)
			{

				iCharCnt++;
				tmpLine = realloc(tmpLine, sizeof(char) * iCharCnt + 1);

				if(tmpLine != NULL)
				{
					if(blnCpy == MFALSE)
					{
						strcpy(tmpLine, tmpChar);
						blnCpy = MTRUE;
					}
					else
					{
						strcat(tmpLine, tmpChar);
					}	
					}
					else
					{
						errDbg(__func__, __LINE__, "Speicher fuer temporaere Zeile konnte nicht erweitert werden");
					}

				}
		}
		else
		{
			appendLineToList(pList, tmpLine, pAnzLines);
			(*pAnzLines)++;
			iCharCnt = 0;
			free(tmpLine);
			tmpLine = NULL;

		}

	}while(iChar != EOF);

	free(tmpLine);

	return EXIT_SUCCESS;
}

FILE *openFile(char *pFilePath)
{
	/*Variablen*/
	FILE *tmpFile;

	printFunc(__func__);

	/*Öffnen der Datei*/
	tmpFile = fopen(pFilePath, "r");

	/*Überprüfen ob die Datei geöffnet werden konnte*/
	if(tmpFile != NULL)
	{
		/*Dateipointer zurückgeben*/
		return tmpFile;
	}
	else
	{
		/*NULL = Datei konnte nicht geöffnet werden*/
		return NULL;
	}
}

int main(int argc, char **argv)
{
	/*Variablen*/
	FILE *XMLFile;
	char **lstLines;
	int iAnzLines = 0;

	printFunc(__func__);

	/*Logik und Ausgaben*/	
	XMLFile = openFile("/home/fabi/Programmierung/C_C++/Reader/text.txt");

	//fprintf(stdout, "XML-Parser\n");

	if( XMLFile != NULL )
	{
		/*Liste initialisieren*/
		lstLines = NULL;
		/*Zeilen auslesen*/
		readLines(&lstLines, &iAnzLines, XMLFile);

		/*Zeilen auf der Konsole ausgeben*/
		printLineList(&lstLines, &iAnzLines);

	}
	else
	{
		errDbg(__func__, __LINE__, "Datei konnte nicht geoffnet werden");
		return EXIT_FAILURE;
	}

	return EXIT_SUCCESS;
}

Zwei unnötige Funktionen (errDbg und printFunc) habe ich weggelassen.

Kann mir vielleicht jemand sagen wo da der Fehler liegt?

Liegt es vielleicht daran, dass ich nur die Speicheradresse die Liste (lstLines) übergebe und dann versuche Speicher zu reservieren?

Vielen Dank für eure Hilfe.

lg,
Fabi++

[CStoll]

Ich bin mir nicht ganz sicher, aber wenn du die Anzahl der Array-Einträge vergrößerst, solltest du am Ende auch deinen Anzahl-Wert erhöhen. Was sonst noch bei dem Code schiefgehen könnte, kann dir dein Debugger vermutlich verraten.

[Fabi++ 0]

CStoll schrieb:

Ich bin mir nicht ganz sicher, aber wenn du die Anzahl der Array-Einträge vergrößerst, solltest du am Ende auch deinen Anzahl-Wert erhöhen. Was sonst noch bei dem Code schiefgehen könnte, kann dir dein Debugger vermutlich verraten.

Erstmal vielen Dank für deine schnelle Antwort.

Aber den Zähler für die Eniträge in dem Array (lstLines) wird ja erhöht.

(*pAnzLines)++;

Zeile 112.

Ich denke daran wird es nicht liegen.
Das habe ich jetzt beim Debuggen schon sehr oft überprüft.
Wie gesagt, er bricht immer nach der dritten Zeile ab, für die zweite Zeile läuft noch alles normal durch.

Das Text-File sieht übrigens so aus:

Z1
Z2
Z3
Z4

Aber ich denke, daran dürfte es nicht liegen.

lg,
Fabi++

[B.B.]

Versuch doch erstmal ein Minimalbeispiel zu programmieren, das deinem Vorhaben entspricht und das du verstehst. Z.B. indem du die einzulesenden Zeilen mit Strings "eins", "zwei", ... simulierst und diese dann in einem dynamisch wachsenden char** Array oder in einer Liste etc. speicherst. Das Programm kannst du, wenn das Minimalbeispiel funktioniert, quasi drum herum bauen.

Als Ansatz meine ich in etwa so etwas in der Art:

char* list[] = {"eins", "zwei", "drei", "vier", "fuenf", "sechs", "sieben", "acht", NULL };

char** plist_create(unsigned* n)
{
	char** new_list = NULL;
	int i = 0;

	while ( NULL != list[i] )
	{
		new_list = malloc ....
		...
		strcpy(new_list[i], list[i]);
		(*n)++; // 
		i++;
	}

	return new_list;
}

void view_plist( char** p, unsigned n )
{
	// testausgabe
}

void free_plist(char** p)
{

}

int main()
{
	unsigned n = 0;
	char** plist = plist_create(&n);
	view_plist(plist, n);
	free_plist(plist);	
	return 0;
}

Btw. sind m.M.n. Dreifachzeiger viel zu kompliziert, müssen nicht sein.

[Fabi++ 0]

@B.B.:
Danke für die Antwort.
Das Beispiel ist ja schon relativ klein und das auslesen der Datei funktioniert ja auch einwandfrei.

Btw. sind m.M.n. Dreifachzeiger viel zu kompliziert, müssen nicht sein.

Nenn mir ne brauchbare alternative, dann verwende ich die ;).
Sollte aber auch dynamisch sein!

lg,
Fabi

[Wutz]

void appendLineToList(char ***pList, char* pLine, int *pAnzLines)
{
	int iNewSize = (*pAnzLines) + 1; /*Klammer überflüssig*/

	/*Debug-Informationen*/
	printFunc(__func__);

	/*Speicher für weiteren Eintrag in der Liste reservieren*/
	*pList = realloc( *pList, sizeof( char* ) * iNewSize + 1);/*+1 überflüssig*/

	if(*pList != NULL)
	{
		*pList[(*pAnzLines)] = NULL;/*ganze Zeile überflüssig*/
		*pList[(*pAnzLines)] = malloc( sizeof( char ) * strlen(pLine) + 1);/*hier fehlt die Klammerung, d.h. richtig ist: (*plist)[*pAnzLines], sizeof(char) ist überflüssig da immer 1*/

		if(*pList[(*pAnzLines)] != NULL)/*Klammer fehlt s.o.*/
		{
			strcpy(*pList[(*pAnzLines)], pLine);/*Klammer fehlt s.o.*/
			fprintf(stdout, "Inhalt der Liste %s Position: %d\n", *pList[*pAnzLines], *pAnzLines);/*Klammer fehlt s.o.*/
		}
		else
		{
			errDbg(__func__, __LINE__, "Speicher fuer String reservieren (malloc)");/* __func__ nur C99 */
			return EXIT_FAILURE;
		}
	}
	else
	{
			errDbg(__func__, __LINE__, "Speicher fuer weiteren Eintrag in der Liste reservieren (realloc)");
			return EXIT_FAILURE;
	}

}

int readLines(char ***pList, int *pAnzLines, FILE *pFile)
..
		if(iChar != EOL)/* besser weil portabel ist '\n' statt 10 */

Ich habe jetzt mal aufgehört, wären sonst mehr Anmerkungen als bemängelter Code geworden, das ganze Geraffel lässt sich ohne großen Aufwand auf <50% bringen und dann auch fehlerfrei...

[CStoll]

Fabi++ schrieb:

CStoll schrieb:

Ich bin mir nicht ganz sicher, aber wenn du die Anzahl der Array-Einträge vergrößerst, solltest du am Ende auch deinen Anzahl-Wert erhöhen. Was sonst noch bei dem Code schiefgehen könnte, kann dir dein Debugger vermutlich verraten.

Erstmal vielen Dank für deine schnelle Antwort.

Aber den Zähler für die Eniträge in dem Array (lstLines) wird ja erhöht.

(*pAnzLines)++;

Zeile 112.

Na, das ist ein Beispiel für schlechte Code-Struktur - normalerweise hätte ich das in der Append-Funktion erwartet.

Aber was noch problematisch sein könnte: Du setzt bInCopy nicht wieder zurück, wenn du den temporären String freigibst - das heißt in der zweiten Dateizeile setzt du strcat() auf nicht-initialisierten Speicher an.

[B.B.]

Alternativ kann man char** zurückgeben und im Fehlerfall errno setzten, z.B.

[Wutz]

Das Setzen von prozessglobalen Variablen, zu denen auch errno gehört, führt zu undefiniertem Verhalten in Multithreadumgebungen.

[Wutz]

B.B. schrieb:

char* list[] = {"eins", "zwei", "drei", "vier", "fuenf", "sechs", "sieben", "acht", NULL };

char** plist_create(unsigned* n)
{
	char** new_list = NULL;
	int i = 0;

	while ( NULL != list[i] )
	{
		new_list = malloc ....
		...
		strcpy(new_list[i], list[i]);
		(*n)++; // 
		i++;
	}

	return new_list;
}

void view_plist( char** p, unsigned n )
{
	// testausgabe
}

void free_plist(char** p)
{

}

int main()
{
	unsigned n = 0;
	char** plist = plist_create(&n);
	view_plist(plist, n);
	free_plist(plist);	
	return 0;
}

Btw. sind m.M.n. Dreifachzeiger viel zu kompliziert, müssen nicht sein.

Das malloc möchte gern ein realloc sein, woran soll "free_plist" die Anzahl der Elemente erkennen?
Wenn man Zeiger nicht verstanden hat, sind auch Einfachzeiger ein Problem, wenn man Zeiger verstanden hat, sind auch *** kein Problem.

[B.B.]

Sag mal, warum kopierst du denn den ganzen Code?

Wutz schrieb:

Das malloc möchte gern ein realloc sein, woran soll "free_plist" die Anzahl der Elemente erkennen?

Das bleibt dem Programmierer überlassen, dieses festzustellen. Da ist eben noch Raum für didaktische AHA-Effekte

Wutz schrieb:

Wenn man Zeiger nicht verstanden hat, sind auch Einfachzeiger ein Problem, wenn man Zeiger verstanden hat, sind auch *** kein Problem.

Ja, ist klar, frei nach dem Motto: warum einfach, wenn es auch umständlich geht.

[seldon]

errno ist heute in der Regel thread-local, um genau dieses Problem zu umgehen. Streng nach ISO (und auch nach POSIX) kann man sich darauf allerdings nicht verlassen.

Ansonsten würde ich mir das Rumfuhrwerken mit realloc komplett sparen und eine Art dynamisches Struct aufbauen (d.h. flach im Speicher). Die einfachste Methode ist hier ein Zwei-Pass-Verfahren dieser Art:

#include <assert.h>
#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

struct string_array {
  size_t size;
  char **data;
};

struct string_array *get_file_contents(char const *fname) {
  char buf[16 * 1024];
  char *p;
  FILE *fd;
  size_t i;
  size_t line_count = 1;
  size_t file_size  = 0;
  size_t bytes_read;
  struct string_array *result;

  fd = fopen(fname, "r");
  if(fd == NULL) return NULL;

  /*
   * Zwei-Pass-Verfahren: Erst die Zeilen zählen, um dann den großen Plan (s.u.)
   * umsetzen zu können.
   */
  do {
    p = buf;

    bytes_read = fread(buf, 1, sizeof(buf), fd);
    file_size += bytes_read;

    while((p = memchr(p, '\n', buf + bytes_read - p))) {
      ++p;
      ++line_count;
    }
  } while(bytes_read != 0);

  rewind(fd);

  /*
   * Der Plan ist, den Array-Kopf, den Index und die Daten direkt hintereinander in
   * einen großen Speicherblock zu legen, d.h.
   *
   * | size | data | i1 | i2 | i3 | ... | "foo" | "bar" | "baz" | ... |
   * +------+------+----+----+----+-----+-------+-------+-------+-----+
   *            |___^ |    |    |         ^       ^       ^
   *                  |____|____|_________|       |       |
   *                       |____|_________________|       |
   *                            |_________________________|
   */
  result = malloc(sizeof(struct string_array) + /* Kopf   */
                  sizeof(char*) * line_count  + /* Index  */
                  file_size + 1);               /* Inhalt */
  if(result == NULL) return NULL;

  result->size    = line_count;
  result->data    = (char**) (result + 1);
  result->data[0] = (char* ) (result->data + line_count);
  p = result->data[0];

  assert(file_size == fread(p, 1, file_size, fd));
  fclose(fd);
  p[file_size] = '\0';

  for(i = 1; i < line_count; ++i) {
    p = memchr(p, '\n', result->data[0] + file_size - p); /* Wir wissen, dass das nie NULL wird */
    *p++ = '\0';
    result->data[i] = p;
  }

  /* Eine leere letzte Zeile wird per Konvention nicht als Zeile aufgefasst. */
  if(*result->data[result->size - 1] == '\0') --result->size;

  return result;
}

int main(int argc, char *argv[]) {
  struct string_array *file_contents;
  size_t i;

  file_contents = get_file_contents(argc > 1 ? argv[1] : __FILE__);
  if(file_contents == NULL) return -1;

  for(i = 0; i < file_contents->size; ++i) {
    puts(file_contents->data[i]);
  }

  /*
   * Freigabe auf einen Schlag hier.
   */
  free(file_contents);

  return 0;
}

Der Hauptvorteil dieser Variante ist die denkbar einfache Speicherverwaltung - die Fehlerbehandlung beschränkt sich auf Zeile 57, und die Freigabe ist sichtbar unkompliziert. Es ist auch, insbesondere bei kurzen Zeilen, ein geringerer Overhead zu erwarten - sowohl an Laufzeit als auch an Speicher (weniger Speicherblöcke, die malloc und free kennen müssen). Ein potentieller Nachteil ist natürlich, dass die Datei zweimal gelesen werden muss; auf einem Kassettenlaufwerk wäre das also nicht empfehlenswert. Wo Speichermangel nicht zu erwarten ist, kann man vor dem ersten Durchlauf die Datei komplett in den Speicher laden und das Problem damit umgehen (stat ist hier ein Freund); wie bei der realloc-Variante muss man dann aber (wenn auch nicht in ganz so großem Ausmaß) mit Heap-Fragmentierung rechnen.

[Wutz]

B.B. schrieb:

Sag mal, warum kopierst du denn den ganzen Code?

Damit dein auf einem Niveau mit JW liegender "Beispielcode" der Nachwelt als abschreckendes Beispiel erhalten bleibt.

B.B. schrieb:

Das bleibt dem Programmierer überlassen, dieses festzustellen. Da ist eben noch Raum für didaktische AHA-Effekte

Platte Ausrede.
Bei view_plist hast du schließlich auch die Größe mit übergeben und 3 Zeilen später dann vergessen?! Dir fehlt der Überblick für solche Konstellationen.

[Fabi++ 0]

Vielen Dank für Eure Hilfe.
Das Problem konnte mit den Verbesserungen von Wutz umgesetzt werden.

lg,
Fabi++