Problem beim korrekten Einlesen einer Datei, EOF wird übergangen

Berufspenner

Hi

Ich schreibe gerade für die Uni ein kleines Programm, das DICOM-Dateien (digitale CT-Bilder) einlesen und bearbeiten soll. Leider habe ich ein da noch ein Problem mit dem Beenden des Lesevorganges. Ich habe extra eine Funktion geschrieben, die eine Datei öffnet und die einzelnen Bildinformationen in eine Matrix (zweidimensionales Array) einließt. Das klappt auch soweit, nur reagiert der Leseprozess nicht auf die Abfrage nach EOF. Hier mal mein Code:

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

short** ct_import(char* filename, int size)
{
    FILE* file_in;
    short i;
    short** _ct;

    if((file_in = fopen(filename, "rb")) == NULL)
    {
        printf("Datei konnte nicht ge%cffnet werden\n", 246);
        exit(-1);
    }

    if((_ct = (short**) calloc(size, sizeof(short))) == NULL)
    {
        printf("Speicher konnte nicht alloziert werden\n");
        free(_ct);
        exit(-1);
    }

    for(i = 0; i < size; i++)
    {
        _ct[i] = (short*) calloc(size,sizeof(short));

        if(_ct[i] == NULL)
        {
            printf("Speicher konnte nicht alloziert werden\n");

            for(i = 0; i < size; i++)
            {
                free(_ct[i]);
            }
            free(_ct);
        }
    }

    //Vom Ende aus hinter der Header springen
    fseek(file_in,-size*size,SEEK_END);

    //Blockweise einlesen in das Feld
    while(!feof(file_in))
    {
        for(i = 0; i < size; i++)
        {
            fread(_ct[i], sizeof(short), size, file_in);
        }
    }

    fclose(file_in);
    return _ct;
}

Wenn ich diese Funktion in ein kleines Testprogramm

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

int main(int argc, char** argv)
{
    int i,j;
    if(argc != 2)
    {
        printf("Usage: dicom <importfile>\n");
        exit(1);
    }

    short** input = ct_import(argv[1],512);

    for(i = 0; i < 512; i++)
    {
        for(j = 0; j < 512; j++)
        {
            printf("%i\n",input[i][j]);
        }
        printf("\n");
    }

    return 0;
}

einbinde und durchlaufen lasse, dann gibt er mir auch schön die Werte der Bildpunkte aus, nur leider haut er irgendwann nur noch Nullen raus, da er offensichtlich am Ende der Datei angekommen ist, und zu guter letzt kommt es noch zu einem Speicherüberlauf. Ließt fread() mehr ein, als es darf und verursacht den Fehler? Ich weiß gerade nicht so richtig, wo der Fehler steckt.

Tachyon

Spontan fällt mir schon mal das hier auf:

if((_ct = (short**) calloc(size, sizeof(short))) == NULL) //sizeof(short) != sizeof(short*) !! (zumindest mit hoher wahrscheinlichkeit)
    {
        printf("Speicher konnte nicht alloziert werden\n");
        free(_ct);
        exit(-1);
    }

Da gibts noch einiges zu meckern, vor allem, bei den Freigaben im Fehlerfall, aber nun gut...

Berufspenner

Tachyon schrieb:

Spontan fällt mir schon mal das hier auf:

Ok, hast recht. Nun bringt er mir schonmal keinen Bufferoverflow mehr, was ja schonmal viel wert ist und er beendet sich von selber.

Tachyon schrieb:

Da gibts noch einiges zu meckern, vor allem, bei den Freigaben im Fehlerfall, aber nun gut...

Bis jetzt ist es halt nur rein experimentel zusammengeklatscht. Aber trotzdem kannst du ruhig meckern bzw. noch sagen, was dir für ein endgültiges Programm einfällt. Dadurch kann das Ergebniss nur besser werden

das 'feof' kommt bei dir nur dran, wenn 'fread' size-mal aufgerufen wurde, also zu selten. dabei kann's passieren, dass er über die datei hinausliest. ansonsten: die casts vor calloc/malloc etc. sind überflüssig, haben aber mit deinem fehler nix zu tun.

Berufspenner

~fricky schrieb:

das 'feof' kommt bei dir nur dran, wenn 'fread' size-mal aufgerufen wurde, also zu selten. dabei kann's passieren, dass er über die datei hinausliest.

Da sieht man mal wieder den Wald vor lauter Bäumen nicht...ja, du hast recht. Die Abfrage von feof muss natürlich in die Bedinung der for-Schleife mit rein. Jetzt hört er mir auch an der richtigen Stelle auf

Tachyon

Okay, ich kommentiere mal alles durch (ich habe echt zu viel Zeit):

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

short** ct_import(char* filename, int size)
{
    FILE* file_in;
    short i;
    short** _ct; //sollte auf NULL gesetzt werden

    if((file_in = fopen(filename, "rb")) == NULL)
    {
        printf("Datei konnte nicht ge%cffnet werden\n", 246);
        exit(-1);
    }

    if((_ct = calloc(size, sizeof(*_ct))) == NULL) //der Cast ist ueberfluessig, besser Variable als Typ in sizeof
    {
        printf("Speicher konnte nicht alloziert werden\n");
        //wo wird close(file_in) gemacht?
        free(_ct);
        exit(-1);
    }
    //jetzt sollten alle Elemente von _ct auf NULL gesetzt werden

    for(i = 0; i < size; i++)
    {
        _ct[i] = calloc(size,sizeof(**_ct)); //der Cast ist ueberfluessig, besser Variable als Typ in sizeof

        if(_ct[i] == NULL)
        {
            printf("Speicher konnte nicht alloziert werden\n");

            for(i = 0; i < size; i++)
            {
                free(_ct[i]);  //nach dem free auf NULL setzen
                //was passiert hier, wenn 20 von 50 Elementen alloziiert wurden
                //der Rest aber noch nicht, die _ct[i]-Elemente aber nicht
                //wie oben angemerkt auf NULL gesetzt wurden? 
            }
            free(_ct); //wird immer wieder geloescht (size x)!
        }
        //wo wird close(file_in) gemacht?
        //kein exit() -> Programm macht potentiell auf Schrottpointern weiter
    }

    //Vom Ende aus hinter der Header springen
    fseek(file_in,-size*size,SEEK_END);

    //Blockweise einlesen in das Feld
    while(!feof(file_in))
    {
        for(i = 0; i < size; i++)
        {
            //ob das so gut ist?!
            fread(_ct[i], sizeof(short), size, file_in);
        }
    }

    fclose(file_in);
    return _ct;
}