Gibts was besseres als fgets?

[CJens]

Hallo.

Ich möchte eine sehr große Textdatei bearbeiten. Die Datei hat eine Größe von 1.6 GB. fgets versagt, wobei es natürlich auch an etwas anderem liegen kann.
Bei anderen, kleineren Dateien funktioniert das Programm aber und liefert auch korrekte Ergebnisse.

Folgender Code:

int ReadNastranFile(char* filename, TDomain* fpDomain, unsigned int nDomain, unsigned int Output){

	//Standard counter
	unsigned long i=0, j=0, k=0, l=0, m=0, n=0, ReadingLine=0,fi=0;

	//Container for meshfile
	char **Puffer=NULL, *line=NULL;

	//Container for specific line
	line = (char*)calloc(81, sizeof(char));

	//File status struct
	struct stat FileStatus;

	//Pointer to meshfile
	FILE* NMF=NULL;

	//Checking if meshfile exists...
	if(FileExist(filename)!=0){
		if(Output){
			printf("\n\n\tERROR: Unable to open %s for reading...\n\tApplication terminates with returncode 1.\n\n\tPress any key to exit...", filename);
			getch(); 
			printf("\n\n\n\t\t\tYou'll get it next time...!");
			Sleep(2000);
		}
		return 1;
	}

	//Getting Size of Meshfile
	stat(filename, &FileStatus);
	nLines = FileStatus.st_size/(LineLength*sizeof(char))+2;
	if(Output){printf("\t%lu lines to read!\n", nLines);}

	//Allocating memory for meshfile
	Puffer = (char **)calloc(nLines, sizeof(char *));
	j=sizeof(char);
	do{
		Puffer[i] = (char *)calloc(LineLength, j);
	}while(++i<nLines); i=0; j=0;

	//Open meshfile for reading
	NMF = fopen(filename, "r");

	if(Output){printf("\tPuffering mesh information...");}
	//Reading meshfile to puffer
	while(fgets(line, LineLength, NMF)){ /*ERROR*/
		ReadingLine++;
                /*Und so weiter....
                ...und sofort!*/

Der Fehler wird in Zeile 47 erzeugt.

Die Textdatei beginnt wie folgt:
$ Generated by ICEMCFD - NASTRAN Interface Vers. 14.0.0
$ Nastran input deck
SOL 103
CEND
$
BEGIN BULK

Jede Zeile hat eine Länge von 81

Bereits beim ersten Schleifendurchlauf versagt das Programm.

Hat jemand eine Ahnung, woran das liegen könnte?

Ich verwende Visual Studio 2013 den VCPP Compiler.
Windows 7 - 64 Bit
Genügend Hauptspeicher ist vorhanden.

Vielen Dank,
CJens

Nachtrag: Auchso, das Programm wird im Debugger unterbrochen und VC++ wechselt in seiner Oberfläche zu einer Meldung:
"Microsoft Visual Studio C Runtime Library hat einen schwerwiegenden Fehler in VCPPF2D.exe gefunden."

Dann schreibt er, dass die Debugginformationen nicht vorhanden sind.

[Wutz]

Das Programm versagt, weil du versagst. fgets ist fehlerfrei.
- du prüfst fopen nicht
- du verwendest mal LineLength und mal 81
- du castest malloc
- unter Windows gibt es für EOF-Erkennung bei Textstreams Besonderheiten
- deine "Text"-Datei darf kein Binär-0 enthalten
- jede Zeile soll 81 Zeichen lang sein? - das sieht in deinem Beispiel nicht so aus
- ein Zeilenstring von 81er Länge kann nur 80 Zeichen aufnehmen
- du scheiterst an simplen Grundlagen und gibst fgets die Schuld
- u.v.a.m.

[DirkB]

- du überprüfst die Rückgabewerte von malloc nicht auf Fehler.

[SeppJ]

Wutz schrieb:

- unter Windows gibt es für EOF-Erkennung bei Textstreams Besonderheiten

?

[Wutz]

Probiers aus, wenn du es nicht glaubst. Ist in MSDN dokumentiert.

#define _CRT_SECURE_NO_WARNINGS
#include <stdio.h>

#define CTRLZ "\x1a"

int main()
{
  char t[100], s[] = "hello" CTRLZ "world"; /* Datei enthält ^Z */
  FILE *f = fopen("bla.txt", "wb");  fwrite(s, sizeof s, 1, f);  fclose(f);

  /* Textfile unter Windows lesen*/
  f = fopen("bla.txt", "r");
  if( fgets(t, 100, f)) puts(t);  /* Überraschung, ASCII[26] wird als EOF interpretiert */
  fclose(f);

  /* Binärfile unter Windows lesen*/
  f = fopen("bla.txt", "rb");
  if (fgets(t, 100, f)) puts(t);
  fclose(f);  

  return 0;
}

Programm schrieb:

hello
hello→world

[SeppJ]

Cool, das habe ich irgendwie verdrängt. Als ich vor ~20 Jahren meine ersten Kontakte mit MS-DOS hatte, habe ich erfahren, dass es so ist. Später habe ich mir im laufe der Zeit eingeredet, dass ich das wahrscheinlich nur falsch verstanden haben muss, weil niemand bei klarem Verstand solch ein Verhalten einbauen würde.

[Gast]

Ich habe vor Kurzem extra ein Programm geschrieben, um so Sachen wie ^Z zu entfernen, weil manche andere Programme (u.a. john the ripper) damit nicht umgehen können.

Programm konvertiert auch 15 gb große Dateien problemlos. Dauert dann aber ein bißchen............
Und auch 1000 files gleichzeitig (nacheinander).

static bool IsControlCode( wint_t i ) { return( iswcntrl( i ) != 0 && i != _T('\r') && i != _T('\n') ); }
static bool IsControlCodeOrSpace( wint_t i ) { return( (iswcntrl( i ) != 0 && i != _T('\r') && i != _T('\n')) || i == _T(' ') ); }

...

wchar_t* pbegin = m_pContent_str;
wchar_t* pend = m_pContent_str + characters_read;
if( !m_rem_spaces )
	pend = std::remove_if (pbegin, pend, IsControlCode );
else
	pend = std::remove_if (pbegin, pend, IsControlCodeOrSpace );
std::streamsize len2write = static_cast<std::streamsize>(pend - pbegin);

[dachschaden_off]

Das wusste ich sogar.
Ich hatte vor Jahren einen Savegame-Editor in C geschrieben, und beim Auslesen der Datei brach das Programm bei einem Pfeil nach rechts ('→', Zeichen für 0x1a/26 unter Codepage 437) ab - ab da wurde nicht mehr gelesen. Und für einen Anfänger, der ich damals noch war, war das definitiv ein WTF-Moment. Das waren damals überhaupt meine ersten Gehversuche mit C.

Und genau wie du, SeppJ, habe ich das mit den Jahren auch verdrängt, weil ich dann an Linux kam. Und der Grund, warum es das 'b'-Flag beim fopen gab, habe ich mir so erklärt, dass die Laufzeitumgebung sonst aus \r\n ein \n macht, um den Code portable zu halten. Mit der gleichen Begründung übrigens: "Warum würde man SOWAS implementieren?"

Und noch was: dieses Verhalten beim 'b'-Flag ruft meiner Erfahrung nach mehr Probleme hervor. Weil die Programmierer denken, dass sie schlauer sind, weil sie wissen, dass es unterschiedliche Zeilenumbrüche auf den verschiedenen Systemen gibt. Dann starten sie beim Windows beispielsweise immer ein Byte später, aber das bräuchten sie gar nicht.

Äh, nun, zum Thema:

int ReadNastranFile(char* filename, TDomain* fpDomain, unsigned int nDomain, unsigned int Output)
{

    /*Wenn du noch nach einem Monat weißt, wovor die Variablen hier stehen:
    **Glückwunsch. Ich tät's nicht.*/
    unsigned long i=0, j=0, k=0, l=0, m=0, n=0, ReadingLine=0,fi=0;

    //Container for meshfile
    char **Puffer=NULL, *line=NULL;

    /*Abgesehen davon, dass dir ein strengerer Compiler den Kopf abreißen
    **würde, weil du zwischen Definitions- und Code-Blöcken hin- und
    **herspringst ... ernsthaft? Du reservierst für so eine kleine Menge
    **Speicher auf dem Heap? Das ist nicht die unnützeste Verwendung des Heaps,
    **die ich je gesehen habe, aber schon nah dran.*/
    /*line = (char*)calloc(81, sizeof(char));*/

#define LINE_SIZE 80
    char line[LINE_SIZE+1];

    //File status struct
    struct stat FileStatus;

    //Pointer to meshfile
    FILE* NMF=NULL;

    /*Öhm ... OK, hier hätte ich mal eine Frage, weil ich das in meinem Code
    **so gemacht habe ... ich prüfe nicht, ob die Datei existiert, sondern
    **ich state die Datei einfach. Und wenn das nicht funktioniert, lass
    **ich es sein. Einen zusätzlichen Check benötigt man dann doch nicht.
    **Oder?*/
    if(FileExist(filename)!=0)
    {
        /*Codus verschwindibus.*/
    }

    /*Genau deswegen. stat gibt einen Returncode zurück, ob das jetzt geklappt
    **hat. Das ist außerdem viel sicherer, weil es sein kann, dass die Datei
    **zwar existiert, aber nicht lesbar ist.
    **
    **Dein Code kann an zwei Stellen hochgehen: Wenn du nicht staten kannst,
    **oder wenn du die Datei nicht zum Lesen öffnen kannst. Aus welche Grund
    **dies nicht funktioniert (was dir übrigens vom Fehlercode gesagt wird),
    **darauf prüfst du nicht vorher, sondern bei einem ordentlichen Fehlercode
    **hörst du einfach auf mit deiner Funktion.
    **
    **Wenn's du portable haben willst (nicht auf jedem System gibt es stat -
    **da kannst du dann eventuell verloren haben, und auf Linux und WIndows
    **sind die beiden Calls subtil anders), kannst du auch ein Makro für den
    **Call erstellen. Habe ich bei meinen Libs auch gemacht. Funktioniert ganz
    **gut.*/
    stat(filename, &FileStatus);

    /*Äh, was?
    **LineLength? Die ... die Variable kenn ich ja gar nicht.
    **(Und jetzt weißt du, warum man das über Konstanten wie LINE_SIZE macht) ...
    **Warte mal. Sagen wir mal, die Datei ist 400 Bytes groß, hat aber 50
    **Zeilen. Mit deiner Berechnung hast du aber nur 400/80*1+2 = Platz für 7
    **Zeilen. Ja, super. Klassischer Heap-Overflow incoming ...*/
    nLines = FileStatus.st_size/(LINE_SIZE*sizeof(char))+2;
    if(Output){printf("\t%lu lines to read!\n", nLines);}

    /*Hier würde ich einen Check machen - wenn die Datei nicht größer als 8000
    **Zeichen ist, würde ich versuchen, über _alloca den Speicher auf dem
    **Stack zu holen. Ansonsten ist malloc in Ordnung ... äh, Augenblick mal,
    **calloc? Wieso? Du schreibst doch gleich eh wieder in die Zeiger ... das
    **Nullen brauchst du gar nicht, und es kann auch teuer werden:
    **-> https://www.c-plusplus.net/forum/329755.
    **Und warum lässt du das Ganze nicht als eindimensionales Array und weißt
    **jeder Zeile nur eine feste Größe zu? Es gibt ja auch realloc.*/
    Puffer = (char **)calloc(nLines, sizeof(char *));

    /*Jesus, und warum das? sizeof wird zur Compilierzeit ausgewertet. Da
    **musst du nichts zwischenspeichern. Bei dynamischen Werten, beispielsweise
    **bei strlen, da ist das nicht verkehrt, aber hier so unnütz wie ein
    **Fahhrad in einem See.
    **Moment mal ... j initialisierst du hier, aber i oben? Wieso? Wenn ich
    **mir den Code durchlese, will ich nicht etliche Zeilen nach oben gehen
    **müssen, um die letzte Zuweisung im Kopf zu haben.*/
    i=0;
    j=sizeof(char);
    do
    {
        Puffer[i] = (char *)calloc(LineLength, j);
    }
    while(++i<nLines);
    i=0;
    j=0;

    /*Den Rest spare ich mir, dafür müsstest du dann zahlen ...*/

Und dann wunderst du dich, dass fgets rumspackt?
Wutz: +((size_t)-1) ;

[CJens]

Wutz schrieb:

Das Programm versagt, weil du versagst. fgets ist fehlerfrei.
- du prüfst fopen nicht
- du verwendest mal LineLength und mal 81
- du castest malloc
- unter Windows gibt es für EOF-Erkennung bei Textstreams Besonderheiten
- deine "Text"-Datei darf kein Binär-0 enthalten
- jede Zeile soll 81 Zeichen lang sein? - das sieht in deinem Beispiel nicht so aus
- ein Zeilenstring von 81er Länge kann nur 80 Zeichen aufnehmen
- du scheiterst an simplen Grundlagen und gibst fgets die Schuld
- u.v.a.m.

Ok, ich entschuldige mich hiermit höflichst beim Entwickler von fgets und verweise auf "...es kann natürlich auch an etwas anderem liegen..."

Ich habe jetzt eine Prüfung für fopen eingebaut - ist != NULL.
LineLength ist 81
Was ist "casten"?
Im Texteditor ist der letzte Cursor immer auf 81
Eine Zeile mit 81 chars kann 81 chars aufnehmen - ich brauche kein \0 am Ende. Ist kein String. Aber ich habe das Feld auch mal mit 82 char's erstellt... der Fehler bleibt.

Ich arbeite mit dieser Funktion nun schon seit einem halben Jahr... Diese Datei ist die erste, welche diesen Fehler bringt.

Vielen Dank,
CJens

[CJens]

DirkB schrieb:

- du überprüfst die Rückgabewerte von malloc nicht auf Fehler.

Hallo.

Ja, da hast Du recht. Hab ich nun eingebaut...

line != NULL

Die Suche geht weiter...

[CJens]

dachschaden_off schrieb:

Das wusste ich sogar.
Ich hatte vor Jahren einen Savegame-Editor in C geschrieben, und beim Auslesen der Datei brach das Programm bei einem Pfeil nach rechts ('→', Zeichen für 0x1a/26 unter Codepage 437) ab - ab da wurde nicht mehr gelesen. Und für einen Anfänger, der ich damals noch war, war das definitiv ein WTF-Moment. Das waren damals überhaupt meine ersten Gehversuche mit C.

Und genau wie du, SeppJ, habe ich das mit den Jahren auch verdrängt, weil ich dann an Linux kam. Und der Grund, warum es das 'b'-Flag beim fopen gab, habe ich mir so erklärt, dass die Laufzeitumgebung sonst aus \r\n ein \n macht, um den Code portable zu halten. Mit der gleichen Begründung übrigens: "Warum würde man SOWAS implementieren?"

Und noch was: dieses Verhalten beim 'b'-Flag ruft meiner Erfahrung nach mehr Probleme hervor. Weil die Programmierer denken, dass sie schlauer sind, weil sie wissen, dass es unterschiedliche Zeilenumbrüche auf den verschiedenen Systemen gibt. Dann starten sie beim Windows beispielsweise immer ein Byte später, aber das bräuchten sie gar nicht.

Äh, nun, zum Thema:

int ReadNastranFile(char* filename, TDomain* fpDomain, unsigned int nDomain, unsigned int Output)
{
   
    /*Wenn du noch nach einem Monat weißt, wovor die Variablen hier stehen:
    **Glückwunsch. Ich tät's nicht.*/
    unsigned long i=0, j=0, k=0, l=0, m=0, n=0, ReadingLine=0,fi=0;
 

    //Container for meshfile
    char **Puffer=NULL, *line=NULL;
   
    /*Abgesehen davon, dass dir ein strengerer Compiler den Kopf abreißen
    **würde, weil du zwischen Definitions- und Code-Blöcken hin- und
    **herspringst ... ernsthaft? Du reservierst für so eine kleine Menge
    **Speicher auf dem Heap? Das ist nicht die unnützeste Verwendung des Heaps,
    **die ich je gesehen habe, aber schon nah dran.*/
    /*line = (char*)calloc(81, sizeof(char));*/

#define LINE_SIZE 80
    char line[LINE_SIZE+1];

    //File status struct
    struct stat FileStatus;
 
    //Pointer to meshfile
    FILE* NMF=NULL;
   
    /*Öhm ... OK, hier hätte ich mal eine Frage, weil ich das in meinem Code
    **so gemacht habe ... ich prüfe nicht, ob die Datei existiert, sondern
    **ich state die Datei einfach. Und wenn das nicht funktioniert, lass
    **ich es sein. Einen zusätzlichen Check benötigt man dann doch nicht.
    **Oder?*/
    if(FileExist(filename)!=0)
    {
        /*Codus verschwindibus.*/
    }
   
    /*Genau deswegen. stat gibt einen Returncode zurück, ob das jetzt geklappt
    **hat. Das ist außerdem viel sicherer, weil es sein kann, dass die Datei
    **zwar existiert, aber nicht lesbar ist.
    **
    **Dein Code kann an zwei Stellen hochgehen: Wenn du nicht staten kannst,
    **oder wenn du die Datei nicht zum Lesen öffnen kannst. Aus welche Grund
    **dies nicht funktioniert (was dir übrigens vom Fehlercode gesagt wird),
    **darauf prüfst du nicht vorher, sondern bei einem ordentlichen Fehlercode
    **hörst du einfach auf mit deiner Funktion.
    **
    **Wenn's du portable haben willst (nicht auf jedem System gibt es stat -
    **da kannst du dann eventuell verloren haben, und auf Linux und WIndows
    **sind die beiden Calls subtil anders), kannst du auch ein Makro für den
    **Call erstellen. Habe ich bei meinen Libs auch gemacht. Funktioniert ganz
    **gut.*/
    stat(filename, &FileStatus);

    /*Äh, was?
    **LineLength? Die ... die Variable kenn ich ja gar nicht.
    **(Und jetzt weißt du, warum man das über Konstanten wie LINE_SIZE macht) ...
    **Warte mal. Sagen wir mal, die Datei ist 400 Bytes groß, hat aber 50
    **Zeilen. Mit deiner Berechnung hast du aber nur 400/80*1+2 = Platz für 7
    **Zeilen. Ja, super. Klassischer Heap-Overflow incoming ...*/
    nLines = FileStatus.st_size/(LINE_SIZE*sizeof(char))+2;
    if(Output){printf("\t%lu lines to read!\n", nLines);}
 
    /*Hier würde ich einen Check machen - wenn die Datei nicht größer als 8000
    **Zeichen ist, würde ich versuchen, über _alloca den Speicher auf dem
    **Stack zu holen. Ansonsten ist malloc in Ordnung ... äh, Augenblick mal,
    **calloc? Wieso? Du schreibst doch gleich eh wieder in die Zeiger ... das
    **Nullen brauchst du gar nicht, und es kann auch teuer werden:
    **-> https://www.c-plusplus.net/forum/329755.
    **Und warum lässt du das Ganze nicht als eindimensionales Array und weißt
    **jeder Zeile nur eine feste Größe zu? Es gibt ja auch realloc.*/
    Puffer = (char **)calloc(nLines, sizeof(char *));

    /*Jesus, und warum das? sizeof wird zur Compilierzeit ausgewertet. Da
    **musst du nichts zwischenspeichern. Bei dynamischen Werten, beispielsweise
    **bei strlen, da ist das nicht verkehrt, aber hier so unnütz wie ein
    **Fahhrad in einem See.
    **Moment mal ... j initialisierst du hier, aber i oben? Wieso? Wenn ich
    **mir den Code durchlese, will ich nicht etliche Zeilen nach oben gehen
    **müssen, um die letzte Zuweisung im Kopf zu haben.*/
    i=0;
    j=sizeof(char);
    do
    {
        Puffer[i] = (char *)calloc(LineLength, j);
    }
    while(++i<nLines);
    i=0;
    j=0;

    /*Den Rest spare ich mir, dafür müsstest du dann zahlen ...*/

Und dann wunderst du dich, dass fgets rumspackt?
Wutz: +((size_t)-1) ;

Uff. Also, erstmal muss ich Dir beichten, dass ich wohl das bin, was Du beschreibst bei dem Editor, den Du geschrieben hast. Ich bin - und das bitte an alle - Maschinenbauer, kein Informatiker und naja... ich mache das seit 4 Jahren. Aber eben reines Number Crunching.

Nun zum Thema. Worin liegt das Problem, wenn ich den Speicher für "line" über calloc allociere? Ich gebe den ja am Ende wieder frei mit free(). Ist keine Streitfrage, will ja was lernen.

FileExist ist eine Funktion, die genau das machen - die Datei öffnen und wieder schließen und dazwischen prüfen, ob sie denn existiert.

Ja, LineLength hab ich leider rausgelöscht.
Die gesamte Variablendeklaration sieht so aus:

//Standard counter
	unsigned long i=0, j=0, k=0, l=0, m=0, n=0, ReadingLine=0,fi=0;
	unsigned long nLines=0;
	unsigned long fnNode=0, fnBar=0, fnFamily=0, fnElement=0, fnAreaElement=0, fnTri=0, fnQuad=0, fnPoly=0;
	unsigned long NodeEnd=0, BarEnd=0, ElEnd=0;
	unsigned int LineLength = 82, *fpNFamily=NULL;
	unsigned long* fpExt2IntNode=NULL;

Da sind aber viele, ich denke mal verwirrende, structs mit drinnen.

Die Berechnung mit der Zeilenanzahl überprüfe ich gerne nochmal, aber wie ich schon schreib - die Funktion läuft seit einem halben Jahr mit allen anderen Dateien. Außerdem dürfte der Code dann ja nicht beim ersten fgets aussteigen sondern erst etwas später, oder?

j=sizeof(char); Das mache ich, weil die Textdatei etwa 1.6 GB groß ist und gut und gerne auch mal größer sein kann. Das bedeutet aktuell 19 745 158 Zeilen. Ich will nicht 20 Mio. mal die Funktion sizeof aufrufen - das kostet doch Performance, oder?

[DirkB]

CJens schrieb:

Was ist "casten"?

Das Zwangsweise umwandeln von einem Format in ein Anderes, damit der Compiler nicht meckert.
Du wandelst explizit den Rückgabewert von malloc (ein void*) in ein char*.
Das ist aber nicht nötig, da in C ein void* in jeden anderen Zeigertyp implizit wandelbar ist.
Man muss halt wissen warum man das macht.

CJens schrieb:

Im Texteditor ist der letzte Cursor immer auf 81
Eine Zeile mit 81 chars kann 81 chars aufnehmen - ich brauche kein \0 am Ende. Ist kein String.

Aber fgets liefert dir einen nullterminierten String. Ob du willst oder nicht.
Und was der Editor sagt spielt keine Rolle. Die meisten kann man einstellen, wo sie beim scrollen stehen bleiben.

CJens schrieb:

Aber ich habe das Feld auch mal mit 82 char's erstellt... der Fehler bleibt.

Da bekommst du dann aber schwierigkeiten mit

nLines = FileStatus.st_size/(LineLength*sizeof(char))+2;

CJens schrieb:

DirkB schrieb:

- du überprüfst die Rückgabewerte von malloc nicht auf Fehler.

Hallo.

Ja, da hast Du recht. Hab ich nun eingebaut...

line != NULL

Die Suche geht weiter...

Ich hoffe du überprüfst jede Art von malloc, dazu zählen auch calloc und realloc

Wieviel Zeilen gibt denn der Editor an? Deckt sich das mit deinem nLines?
Mach das line (für das fgets) mal größer und gib eine Meldung aus, wenn die Zeile nicht 81 Zeichen lang ist.

[DirkB]

CJens schrieb:

j=sizeof(char); Das mache ich, weil die Textdatei etwa 1.6 GB groß ist und gut und gerne auch mal größer sein kann. Das bedeutet aktuell 19 745 158 Zeilen. Ich will nicht 20 Mio. mal die Funktion sizeof aufrufen - das kostet doch Performance, oder?

sizeof() ist keine eigentliche C-Funktion.
Der Compiler setzt da eine Konstante ein. Da sizeof(char) per Definiotion 1 ist, ist das sowieso überflüssig. Aber letztendlich optimiert der Compiler das weg.

[dachschaden]

CJens schrieb:

Uff. Also, erstmal muss ich Dir beichten, dass ich wohl das bin, was Du beschreibst bei dem Editor, den Du geschrieben hast. Ich bin - und das bitte an alle - Maschinenbauer, kein Informatiker und naja... ich mache das seit 4 Jahren. Aber eben reines Number Crunching.

OK. Aber bevor du anfängst, Fehler in den Funktionen der Laufzeitumgebung zu vermuten, musst du wirklich das Problem nachvollzogen haben. Die Leute reagieren ein wenig empfindlich, wenn du jemandem fehlerhaften Code nachsagst, da musst du das dann auch beweisen können.

EDIT: Übrigens mache ich C nach einer jahrelangen Abstinenz auch wieder erst seit 1 1/4 Jahren. Aber manche Sachen verlernst du nicht.

CJens schrieb:

Nun zum Thema. Worin liegt das Problem, wenn ich den Speicher für "line" über calloc allociere? Ich gebe den ja am Ende wieder frei mit free(). Ist keine Streitfrage, will ja was lernen.

Das mit den verschiedenen Speicherarten (Stack und Heap) ist dir anscheinend schon bekannt. Das Problem ist, dass der Stack sehr leicht zu verwalten ist, er wächst (beinahe kontinuierlich) immer in eine Richung, und eine Veränderung der Größe geht über das Ändern eines einzelnen CPU-Registers (Zugriffe auf Register kosten nichts). Äh, im Idealfall natürlich.

Wenn du jetzt Speicher auf dem Heap reservierst, ist das mit wesentlich mehr Aufwand verbunden. Bei jedem calloc/malloc müssen Listen durchgegangen werden, die den Speicher in Chunks (größere Stücke) unterteilen. Die Laufzeitbibliothek versucht ja auch, die Fragmentierung des Speichers zu vermeiden, indem sie versucht, nicht genutzten Speicher zwischen Chunks ebenfalls zu nutzen. Schließlich arbeiten einige Implementierungen auch so, dass der Speicher, den du zurückbekommst, auf bestimmte Speicheradressen ausgerichtet ist. Spezielle AVX-Instruktionen dürfen bspw. nur auf Speicheradressen angewendet werden, die durch 32 teilbar sind, sonst gibt es einen Bus-Error (dargestellt - zumindest bei mir - durch einen Segfault). Darum musst du dich in der Regel nicht kümmern, aber nichtreservierte Löcher in den Speicher reißen kann das dennoch. Und beim free hast du einen ähnlichen Aufwand.
Außerdem kann es sein, dass dein Prozess gerade keinen Speicher mehr hat. Dann muss die Laufzeitumgebung einen Kernel-Call machen, bei dem neuer Speicher angefordert wird. Das Problem ist: Kernel-Calls sind teuer. Weil der Kernel einen kompletten Kontextwechsel machen muss, alle Register müssen gesichert werden, und manchmal muss der Kernel noch Parameterwerte in seinen eigenen geschützten Speicher kopieren (damit die Werte nicht von einem zweiten Thread geändert werden können). Dann muss neuer Speicher angefordert werden. Faule Kernel (Linux z.B.) sagen einfach an, dass du neuen Speicher bekommst, aber das ist gelogen, die Hardware weiß davon nichts. Erst dann, wenn du versuchst, in diesen Speicher zu schreiben oder davon zu lesen, wird der Speicher oft reserviert - und auch nur dann, weil die Hardware einen Page Fault wirft. Das betrifft dein Programm nicht, weil der Kernel den Page Fault abfängt und dann mal seine Arbeit macht. Aber reell führt das schon zu einer Verzögerung. Deswegen gibt es auch den Trick, nachdem man einen Haufen Speicher vom Betriebssystem gezogen hat, von jeder Seite (meistens so alle 4096 Bytes) ein Byte anzufassen (schreiben oder lesen). Damit zwingt man den Kernel, den Speicher dann auch tatsächlich zu reservieren.

Also kurz: dynamische Speicherverwaltung ist teuer und kompliziert. Man sollte darauf verzichten, wenn es geht. Und 81 Bytes auf dem Stack sind nicht die Welt.

Für eine ausführliche Erklärung verweise ich auf den guten Mann hier. Brauchst du nicht lesen, aber wenn du weiterlernen willst ...

CJens schrieb:

FileExist ist eine Funktion, die genau das machen - die Datei öffnen und wieder schließen und dazwischen prüfen, ob sie denn existiert

Ja, aber wie gesagt, brauchst du das gar nicht. Außerdem kannst du hier auch eine hübsche Race-Condition haben - sagen wir mal, du prüfst, ob die Datei existiert, und die Datei existiert. Und dann wird für ganz kurze Zeit ein anderer Prozess gestartet, und der löscht die Datei. Und dann wird wieder dein Prozess ausgeführt, und du machst stat , und vertraust darauf, dass die Datei immer noch existiert.

Wie gesagt, ich würd den Call komplett weg lassen, und eher prüfen, ob stat und fopen funktioniert hat.

CJens schrieb:

Die Berechnung mit der Zeilenanzahl überprüfe ich gerne nochmal, aber wie ich schon schreib - die Funktion läuft seit einem halben Jahr mit allen anderen Dateien. Außerdem dürfte der Code dann ja nicht beim ersten fgets aussteigen sondern erst etwas später, oder?

Der Fehler ist ja:

Microsoft Visual Studio C Runtime Library hat einen schwerwiegenden Fehler in VCPPF2D.exe gefunden

Das kann alles mögliche sein. Eventuell auch eine Heap Corruption. Vielleicht auch eine unbedeutende Änderung, die alles kaputtmacht. Das kannst du nicht ausschließen. Mögliche Fehlerquellen und Tipps zur Verbesserung der Codequalität haben wir dir ja schon genannt.

CJens schrieb:

j=sizeof(char); Das mache ich, weil die Textdatei etwa 1.6 GB groß ist und gut und gerne auch mal größer sein kann. Das bedeutet aktuell 19 745 158 Zeilen. Ich will nicht 20 Mio. mal die Funktion sizeof aufrufen - das kostet doch Performance, oder?

Siehe DirkBs Antwort. Nein, das kostet dich keinen Cycle.

#include <stdio.h>
#include <stdint.h>

int main(void)
{
        printf("%lu Bits\n",sizeof(uint64_t)*8);
        return 0;
}

Auf meiner Maschine erzeugt der gcc folgenden Assemblercode:

000000000040054a <main>:
  40054a:       55                      push   rbp
  40054b:       48 89 e5                mov    rbp,rsp
  40054e:       be 40 00 00 00          mov    esi,0x40 ; Hier schreibt der
  ; Rechner 64 (0x40) in das esi-Register. Der Compiler hat den Ausdruck
  ; sizeof(uint64_t) durch 8 ersetzt, und 8*8 ist 64 == x040, das kann der
  ; Compiler bereits zur Kompilierzeit ausrechnen.
  400553:       bf 14 06 40 00          mov    edi,0x400614 ; Hier wird die
  ; Adresse des Formatstrings auf den Stack gelegt für den kommenden Call.
  400558:       b8 00 00 00 00          mov    eax,0x0 ; ZUgegebenermaßen,
  ; mein Assembler ist nicht so knorke - aber ich glaube, hier wird der Rück-
  ; gabeplatz leer gemacht, damit nicht eventuell vorhandene Datenleichen
  ; irgendwas kaputtmachen
  40055d:       e8 ce fe ff ff          call   400430 <printf@plt> ; Hier
  ; passiert halt der Call.
  400562:       b8 00 00 00 00          mov    eax,0x0 ; Und weil wir nichts mit
  ; dem Rückgabewert machen, wird er auch wieder gelöscht.
  400567:       5d                      pop    rbp
  400568:       c3                      ret    
  400569:       0f 1f 80 00 00 00 00    nop    DWORD PTR [rax+0x0]

[CJens]

dachschaden schrieb:

OK. Aber bevor du anfängst, Fehler in den Funktionen der Laufzeitumgebung zu vermuten, musst du wirklich das Problem nachvollzogen haben. Die Leute reagieren ein wenig empfindlich, wenn du jemandem fehlerhaften Code nachsagst, da musst du das dann auch beweisen können.

Das ist schon richtig. Ich weiß auch, dass fgets nicht so falsch arbeitet, denn die Funktion arbeitet bei anderen Dateien korrekt. Ich habe aus diesem Grund auch gefragt, ob es etwas besseres gibt. Meine Intention war, dass viele Funktionen durch neuere/bessere (bei der Wortwahl könnt ihr mir gerne helfen) ersetzt wurden. Und ich weiß auch, dass viele Texteditoren bei Dateien mit mehr als ein paar hundert MB in die Knie gehen - und hatte da einen Zusammenhang vermutet. Es ist ja so, dass hier viele Vorschläge gemacht wurden - für die ich sehr dankbar bin - aber die nicht zum Ziel geführt haben.

dachschaden schrieb:

Das mit den verschiedenen Speicherarten (Stack und Heap) ist dir anscheinend schon bekannt. Das Problem ist, dass der Stack sehr leicht zu verwalten ist, er wächst (beinahe kontinuierlich) immer in eine Richung, und eine Veränderung der Größe geht über das Ändern eines einzelnen CPU-Registers (Zugriffe auf Register kosten nichts). Äh, im Idealfall natürlich.

Wenn du jetzt Speicher auf dem Heap reservierst, ist das mit wesentlich mehr Aufwand verbunden. Bei jedem calloc/malloc müssen Listen durchgegangen werden, die den Speicher in Chunks (größere Stücke) unterteilen. Die Laufzeitbibliothek versucht ja auch, die Fragmentierung des Speichers zu vermeiden, indem sie versucht, nicht genutzten Speicher zwischen Chunks ebenfalls zu nutzen. Schließlich arbeiten einige Implementierungen auch so, dass der Speicher, den du zurückbekommst, auf bestimmte Speicheradressen ausgerichtet ist. Spezielle AVX-Instruktionen dürfen bspw. nur auf Speicheradressen angewendet werden, die durch 32 teilbar sind, sonst gibt es einen Bus-Error (dargestellt - zumindest bei mir - durch einen Segfault). Darum musst du dich in der Regel nicht kümmern, aber nichtreservierte Löcher in den Speicher reißen kann das dennoch. Und beim free hast du einen ähnlichen Aufwand.
Außerdem kann es sein, dass dein Prozess gerade keinen Speicher mehr hat. Dann muss die Laufzeitumgebung einen Kernel-Call machen, bei dem neuer Speicher angefordert wird. Das Problem ist: Kernel-Calls sind teuer. Weil der Kernel einen kompletten Kontextwechsel machen muss, alle Register müssen gesichert werden, und manchmal muss der Kernel noch Parameterwerte in seinen eigenen geschützten Speicher kopieren (damit die Werte nicht von einem zweiten Thread geändert werden können). Dann muss neuer Speicher angefordert werden. Faule Kernel (Linux z.B.) sagen einfach an, dass du neuen Speicher bekommst, aber das ist gelogen, die Hardware weiß davon nichts. Erst dann, wenn du versuchst, in diesen Speicher zu schreiben oder davon zu lesen, wird der Speicher oft reserviert - und auch nur dann, weil die Hardware einen Page Fault wirft. Das betrifft dein Programm nicht, weil der Kernel den Page Fault abfängt und dann mal seine Arbeit macht. Aber reell führt das schon zu einer Verzögerung. Deswegen gibt es auch den Trick, nachdem man einen Haufen Speicher vom Betriebssystem gezogen hat, von jeder Seite (meistens so alle 4096 Bytes) ein Byte anzufassen (schreiben oder lesen). Damit zwingt man den Kernel, den Speicher dann auch tatsächlich zu reservieren.

Also kurz: dynamische Speicherverwaltung ist teuer und kompliziert. Man sollte darauf verzichten, wenn es geht. Und 81 Bytes auf dem Stack sind nicht die Welt.

Für eine ausführliche Erklärung verweise ich auf den guten Mann hier. Brauchst du nicht lesen, aber wenn du weiterlernen willst ...

Auf jeden Fall - das ist eine sehr wichtige Information für mich, denn bei dem Programm geht es mir um Performance. Ich versuche auch das nicht in Schleifen zu verwenden sondern den Speicher stets vor der Schleife einmal zu allocieren und anschließend wieder frei zu geben. Aber die Möglichkeit, gewisse Dinge im Cache zu speichern gibt es nicht, oder?

Ich habe jetzt diese Funktion (das bisherige Beispiel ist ja ein Teil eines großen Programms) mal einzeln nachgeschrieben. Anbei der Code - und hier funktioniert das ganze:

#ifndef STDIO_H
	#define STDIO_H
	#include <stdio.h>
#endif

#ifndef SYS_STAT_H
	#define SYS_STAT_H
	#include <sys/stat.h>
#endif

#ifndef STDLIB_H
	#define STDLIB_H
	#include <stdlib.h>
#endif

int main(){
	unsigned long i=0, j=0, nLines=0;
	char nLetter = 81;

	char line[nLetter+1];
	char filename[256];
	char** Puffer=NULL;

	strcpy(filename, "C:/HomeC/CPPF2D_VCPP/Grid/Grid_10Mio.dat\0");

	//File status struct
	struct stat FileStatus;

	//Pointer to meshfile
	FILE* NMF=NULL;

	//Getting Size of Meshfile
	stat(filename, &FileStatus);
	nLines = FileStatus.st_size/(nLetter*sizeof(char));
	printf("File has %lu lines!\n", 19745158);
	printf("%lu lines to read!\n", nLines);

	//Allocating memory for meshfile
	Puffer = (char **)calloc(nLines, sizeof(char *));
	if(Puffer==NULL){printf("ERROR - Unable to allocate memory for string puffer...\n"); return 1;}
	do{
		Puffer[i] = (char *)calloc(nLetter+1, sizeof(char));
		if(Puffer[i]==NULL){printf("ERROR - Unable to allocate memory for string puffer[%lu]...\n", i); return 1;}
	}while(i++<nLines); i=0;

	//Open meshfile for reading
	NMF = fopen(filename, "r");
	if(NMF==NULL){printf("ERROR - Unable to open file %s for reading!\n", filename);return 1;}
	}else{
		//Reading meshfile to puffer
		while(fgets(line, nLetter, NMF)){
			strcpy(Puffer[i], line);
		}

		fclose(NMF);
		printf("File read successfully!\n");
	}

	getch();
	return 0;
}

ABER: Die Berechnung der Zeilenanzahl ist bei dem Programm zu großzügig.
Der Texteditor sagt: 19 745 158 Zeilen
Das Programm sagt: 19 988 925 Zeilen

[DirkB]

CJens schrieb:

Der Texteditor sagt: 19 745 158 Zeilen
Das Programm sagt: 19 988 925 Zeilen

Mal Rückrechnen: 19 988 925 * 81 / 19 988 925 = fast 82

In der Datei sind die Zeilen also 82 Zeichen lang. Diese sind mit \r\n abgeschlossen.
Beim einlesen im Textmodus wird das \r\n in \n gewandelt, so dass nur noch 81 Zeichen bleiben.

Dein Speicher muss aber trotzdem 82 Zeichen belegen, da noch die \0 dazu kommt (fgets macht das).
An fgets musst du die Größe des Buffers übergeben. fgets weiß das da noch die \0 dran kommt und liest u.U. weniger Zeichen ein.
(wenn du 81 angibst, werden maximal 80 Zeichen eingelesen. da wird dann das \n im nächsten Durchgang gelesen

CJens schrieb:

#ifndef STDIO_H
    #define STDIO_H

Includeguards gehören in die Headerdatei. Oder httest du Probleme damit?

CJens schrieb:

strcpy(filename, "C:/HomeC/CPPF2D_VCPP/Grid/Grid_10Mio.dat\0");

Bei Stringliteralen hängt der Compiler automatisch eine \0 an. Darum brauchst du dich nicht kümmern.

Deine feste Vorgabe mit 81 Zeichen ist eine Fehlerquelle und auch nicht elegant.

Steht in den Zeilen bis zum Ende etwas sinnvolles drin?
Kann man da die Leerzeichen am Ende abschneiden?
brauchst du das \n am Ende im Speicher?

[CJens]

Hallo Dirk,

also, ich habe alle Includings in einer eigenen Header Datei - aber für diesen Beispielcode habe ich das nicht gemacht.

Zu dem Casten habe ich dann noch eine Frage - Du sagst, es ist nicht notwendig. Aber ist es problematisch?

Zur Zeilenlänge. Das ist unterschiedlich. Die Datei beinhaltet erstmal eine Liste mit Punkten mit deren 3D Koordinaten. Das heißt, es gibt immer eine x, y und eine z Koordinate, wobei jede Koordinate 8 Zeichen lang ist. Da brauche ich nicht mehr als die erste 41 Zeichen.

Aber dann kommen Elemente. Es gibt Line (2) Elemente, Tri (3), Quad (4), Polygone (x), Tetraeder (4), Pyramiden (5), Prismen (6), Hexaeder (8) und Polyeder (x). Das heißt, dass manche Elemente garnicht in einer, sondern sogar mehreren Zeilen beschrieben werden können. Für ein Element ist der erste Eintrag dessen Nummer, dann dessen Familie (Zugehörigkeit, physikalische Definition) und dann die Punkte, welche dieses aufspannen. Also, für manche Elemente brauche ich alle Zeichen, für manche nicht... was im Endeffekt bedeutet, dass ich (ja) alle Zeichen brauche.

Aber ich möchte das Thema jetzt mal kurz zurückstellen. Ich finde es toll, dass ihr alle so fleißig antwortet. Ich werde jetzt mal versuchen, Eure Tipps einzubauen. Allerdings kann es nicht an der Anzahl der Zeilen liegen, weil der Code ja schon beim ersten fgets aussteigt. Also, da wurde strcpy(line, Puffer[i]) noch garnicht ausgeführt. Für den weitern Code werde ich es berücksichtigen, aber es kann noch nicht der Fehler sein.

Ich versuche jetzt mal, den Code, den ich vor zwei Beiträgen gepostet habe in meinen richtigen Code einzubauen und gebe Euch dann bescheid, ob ich erfolgreich war. Da aber auch meine Familie ihren Anspruch an mich erhebt, wird das wohl nicht vor morgen Nachmittag der Fall sein.

Allein einen schönen dritten Advent und nochmal vielen Dank... und ich sag Euch, ob ich weiter gekommen bin.

Grüße,
CJens.

[CJens]

Hallo,

ich habe den Fehler gefunden. Der lag erstmal nicht im Code. Wenn ich Puffer nicht definiert hatte, dann lief das ganze über fgets hinaus. Puffer selbst konnte nicht komplett erzeugt werden. Das lag daran, dass unter VC++ die Plattform auf Win32 gesetzt war. Ändere ich diese auf x64 läuft das ganze. Und jetzt fiel mir wieder ein, dass man unter 32 bit ja nur 1,x GB Speicher pro Prozess allocieren kann - wenngleich meine Anwendung auch so schon mit über 2GB im Taskmanager angezeigt wurde.

Aber scheinbar hat malloc() seine Probleme bei sehr großen Speicherbereichen (um die 1.6GB) und Win32... Und nein, bevor ich wieder klein gemacht werde, ich möchte nicht der Funktion malloc die Schuld geben

Ist meine Erklärung am Anfang korrekt? Oder hab ich jetzt durch Zufall hier den Fehler gefixt? Ich schrieb schon, dass ich nicht ganz so tief in der Materie stecke...

Grüße und Vielen Dank für die Anregungen,
CJens

[DirkB]

Du hast von malloc keine 1,6 GB Speicher am Stück angefordert.

Für Puffer sind das erstmal deine ca. 20 Mio Zeilen * sizeof(char*) , was bei 32-Bit dann 80 MB sind.

Den Rest holst du dir in kleineren Stückchen a 81 Byte.

Sollte (Windows-)malloc bei 80 MB Probleme bekommen und kein NULL zurückgeben, obwohl der Speicher nicht vorhanden ist, würdest du das sicher im Netz finden.

CJens schrieb:

Wenn ich Puffer nicht definiert hatte,

Wie hast du es denn jetzt gemacht? Ohne Puffer?

[CJens]

Ich habe es etwas anders gemacht. 20 Mio mal calloc hat sehr lange gedauert.
Ich habe es jetzt so gemacht:

//Allocating memory for meshfile
	Puffer = malloc(nLines * sizeof(char *)); 
        if (Puffer == NULL){ printf("ERROR - Unable to allocate %lu bytes for puffer space I.\n", (nLines) * sizeof(char *)); return 1;}

	Dummy = malloc(nLines * (LINELENGTH + 2) * sizeof(char)); 
        if (Dummy == NULL){ printf("ERROR - Unable to allocate %lu bytes for puffer space II.\n", nLines * (LINELENGTH + 2) * sizeof(char)); return 1; }

	Puffer[0] = &Dummy[0];
	for (i = 1; i < nLines; i++){Puffer[i] = &Dummy[i*(LINELENGTH+2)-1];} i=0;

So oder so hatte es mit Win32 nicht geklappt. Er konnte dann zwar noch die Datei in Puffer lesen, aber bei einem späteren malloc ist er dann ausgestiegen. Der Fehler verschwand mit der x64 Plattform.