ReadFile liest einfach nicht richtig!

_matze

Du speicherst zwar die Rückgabe von GetLastError im Fehlerfall, gibst diese aber nicht aus. Was sagt GetLastError denn?

C++ Forumbot

Dieser Thread wurde von Moderator/in pumuckl aus dem Forum C++ (auch C++0x und C++11) in das Forum WinAPI verschoben.

Im Zweifelsfall bitte auch folgende Hinweise beachten:
C/C++ Forum :: FAQ - Sonstiges :: Wohin mit meiner Frage?

Dieses Posting wurde automatisch erzeugt.

Bitte ein Bit

Also wenn das ein gutes Argument wäre, dann könnte ja quasi beliebiger C-Code hier gepostet und vorher mit einem kleinen #include <string> oder class _{}; versehen werden. Im Handumdrehen ist es kein C mehr, und es ist genauso sinnvoll wie das throw/catch im Code vom Supreme.

Ich glaube so manch einer hat schon einen Wutausbruch bekommen weil er in C++ Code nur C Code gesehen hat.

Das Problem ist, dass wenn man schon C++ nutzen möchte, sollte man auch die dazugehörigen Mittel,Funktionen,Klassen nutzen. Es ist halt immer wieder ärgerlich zu sehen, wie man ein char a[512] benutzt, sich dabei die typischen C Probleme wie Effiziens, Buffer Overflow oder auch Speicherlecks einfängt, obwohl es std::string gibt.

Es verbietet aber keiner dass man in seinem C++ Programm die WinAPI benutzt. Nur sollte man, meines Erachtens, schnell wieder zu C++ Stil wechseln.

Bitte ein Bit schrieb:

Das Problem ist, dass wenn man schon C++ nutzen möchte, sollte man auch die dazugehörigen Mittel,Funktionen,Klassen nutzen. Es ist halt immer wieder ärgerlich zu sehen, wie man ein char a[512] benutzt, sich dabei die typischen C Probleme wie Effiziens, Buffer Overflow oder auch Speicherlecks einfängt, obwohl es std::string gibt.

Sehe ich nicht so. C++ ist einfach das bessere C. Selbst wenn man C benutzen möchte benutzt man meist aus Bequemlichkeit einen C++-Compiler. Daher kommt auch diese C/C++-Sprache, bestehend aus C und ein paar ausgewählten Funktionen von C++.

hustbaer

Bitte ein Bit schrieb:

Es ist halt immer wieder ärgerlich zu sehen, wie man ein char a[512] benutzt, sich dabei die typischen C Probleme wie Effiziens, Buffer Overflow oder auch Speicherlecks einfängt, obwohl es std::string gibt.

Was meinst du bitte? Das szFilename was als Parameter an CreateFile übergeben wird, oder den BYTE buffer[MAX_BUFFER_SIZE] ?
Ich sehe in beiden Fällen kein Problem. Wenn szFilename ein char const* ist, dann ist das mMn. die beste Wahl für diesen Parameter ( std::string ist hier unnötig und mMn. auch unsinnig).
Und das Array als Puffer ist genau so sicher oder unsicher wie std::vector<> oder andere "C++ Alternativen". ReadFile nimmt so oder so nur einen Zeiger + Grösse, und wenn man was falsch macht ist man angeschissen. Wie beim String wäre eine Klasse ala std::vector<> hier bloss unnötiger Overhead.

Ich hab' nicht gegen Kritik von Code den man in C++ besser machen könnte, aber bitte nur wenn es auch gerechtfertigt ist.

Was ich dagegen wirklich grässlich finde, ist myMessage - kleine Puffer mit malloc anfordern ist unnötig, hier könnte man wirklich besser std::vector<> verwenden (oder gleich std::stringstream ), und noch dazu wird der Puffer nicht freigegeben.

Bitte ein Bit

Hmm,

der Parameter szFilename ist doch völlig schnuppe. Was ich meinte ist der Parameter puffer. Was machst du wenn eine Datei mehr als MAX_BUFFER_SIZE Bytes groß ist ? Es hört sich doch auch hier wieder nach einer typischen Formulierung: "Eine Datei wird nie größer als x Bytes" an. Woher weis ich das ?

Ganz zu verschweigen dass man hiermit Probleme bekommt, wenn MAX_BUFFER_SIZE in der Größenordnung des Stackgröße kommt. Also was würde man tun ? Man würde den Puffer auf dem Heap legen, was zur Folge hätte dass man peinlichst auf die Freigabe des Speichers achten muss, was bei größeren Funktionen doch ein wenig aufwendig ist.

Würde man stattdessen nun hingehen und die Datei Zeichen für Zeichen einlesen und in ein std::string, std::vector<BYTE> oder was auch immer stecken, wären fast alle Problem erschlagen.

-lowbyte- 0

Bitte ein Bit schrieb:

Man würde den Puffer auf dem Heap legen, was zur Folge hätte dass man peinlichst auf die Freigabe des Speichers achten muss, was bei größeren Funktionen doch ein wenig aufwendig ist.

Egal ob die Funktion gross oder klein ist.

free(memory);

Ist doch pipi...
Das man immer mit so faulen Argumenten kommen muss finde ich auch hässlich.

Bitte ein Bit schrieb:

Hmm,

der Parameter szFilename ist doch völlig schnuppe. Was ich meinte ist der Parameter puffer. Was machst du wenn eine Datei mehr als MAX_BUFFER_SIZE Bytes groß ist ? Es hört sich doch auch hier wieder nach einer typischen Formulierung: "Eine Datei wird nie größer als x Bytes" an. Woher weis ich das ?

Deshalb rufst du ReadFile ja auch solange auf bis das Ende der Datei erreicht ist. Des Weiteren übergiebt man ja auch die Adresse auf einen DWORD Wert, in dem die Funktion dann die tatsächliche Anzahl an eingelesenen Bytes enthält.

lpNumberOfBytesRead [out, optional]

A pointer to the variable that receives the number of bytes read when using a synchronous hFile parameter.

hustbaer

Bitte ein Bit schrieb:

Es hört sich doch auch hier wieder nach einer typischen Formulierung: "Eine Datei wird nie größer als x Bytes" an. Woher weis ich das ?

Hört sich gar nicht so an, er liest das Ding ja Stückweise. Mitdenken.

Ganz zu verschweigen dass man hiermit Probleme bekommt, wenn MAX_BUFFER_SIZE in der Größenordnung des Stackgröße kommt.

Dann macht man das halt nicht. Kleinere Puffer auf den Stack, grössere auf den Heap.

Also was würde man tun ? Man würde den Puffer auf dem Heap legen, was zur Folge hätte dass man peinlichst auf die Freigabe des Speichers achten muss, was bei größeren Funktionen doch ein wenig aufwendig ist.

Ich weiss nicht ob "man" das tun würde. Ich würde es nicht tun. Ich würde std::vector verwenden.

Würde man stattdessen nun hingehen und die Datei Zeichen für Zeichen einlesen und in ein std::string, std::vector<BYTE> oder was auch immer stecken, wären fast alle Problem erschlagen.

OMG ja mach das mal, damit du wie du erst schon geschrieben hast Probleme mit der Effizienz (2x z im Übrigen) vermeidest. BTW: was machst du wenn die Datei mal grösser als der zur Verfügung stehende Speicher ist?

#include <iostream>
#include <windows>
#include <vector> 
using namespace std; 

int main() 
{

HANDLE hfile;
DWORD nbt=0;

hfile =   CreateFile  ( "test.txt", GENERIC_READ , FILE_SHARE_READ , 0,  OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, 0 );
DWORD ret_size = GetFileSize ( hfile , NULL );

vector<char>buf(ret_size);

ReadFile (hfile  , &buf[0] , ret_size  , &nbt , NULL );
cout<< &buf[0] << endl;

system("PAUSE");
return 0;
}

Bitte ein Bit

Hört sich gar nicht so an, er liest das Ding ja Stückweise. Mitdenken.

Nö, so direkt habe ich mich nicht auf das Beispiel bezogen.

Aber für meinen Geschmack sieht es so aus als lese er die letzten MAX_BUFFER_SIZE Bytes aus der Datei, sofern diese größer als MAX_BUFFER_SIZE Bytes ist. Ansonsten liest er die Datei komplett ein.

Zitat:

Ganz zu verschweigen dass man hiermit Probleme bekommt, wenn MAX_BUFFER_SIZE in der Größenordnung des Stackgröße kommt.

Dann macht man das halt nicht. Kleinere Puffer auf den Stack, grössere auf den Heap.

Zitat:

Also was würde man tun ? Man würde den Puffer auf dem Heap legen, was zur Folge hätte dass man peinlichst auf die Freigabe des Speichers achten muss, was bei größeren Funktionen doch ein wenig aufwendig ist.

Ich weiss nicht ob "man" das tun würde. Ich würde es nicht tun. Ich würde std::vector verwenden.

Das ist doch genau das was ich sagen will !

Es ist doch aber so, das einige Leute nach dem Motto "Eine Datei wird nie größer als X Bytes", "Eine Zeile wird nie größer als 80 Zeichen",... programmieren. Und die Probleme, die dadurch entstehen dass die Annahmen nicht erfüllt sind, kann man halt durch STL vermeiden/verschieben. Es ist schon eine Bürde, die man dadurch manchmal sieht, denn ist und bleibt eine unnötige Einschränkung.

BTW: was machst du wenn die Datei mal grösser als der zur Verfügung stehende Speicher ist?

Keine Ahnung. Vermutlich wird man nur einen Teil einlesen (vielleicht Memory Mapped File), oder die Datei nur Zeile für Zeile einlesen (sofern man das eh schon nicht tut). Wobei sich hier wiederrum ein std::vector<> eignen würde, da ich ja nicht die Länge einer Zeile weiss.

Ich mache ja keine Aussage dass man immer die Datei komplett einlesen sollte, sondern ich sage nur dass ich Aussagen der Form XYZ wird nie größer als X Bytes prinzipiell nicht mag, da die Annahme nicht immer stimmt.

Dumme Frage:
Wieso soll jemand eine Datei in X Byte Blöcken einlesen ? Ok, um nicht das RAM mit der Datei zuzumüllen, aus Effiziensgründen weil man beispielsweise die Datei anzeigt (Editor).

Aber wenn ich eine Datei einlesen und interpretieren möchte, muss ich gewisse Elemente (Zeilen, Datenstrukturen, ...) aus der Datei lesen, wobei ich wiederrum auf das Problem der Größenangaben komme. Denn wie groß ist ein Element ? Bzw. wie lese ich ein Element ein wenn es beim ersten Lesenzyklus nur zum Teil eingelesen wurde ? -> Ringpuffer ???

@TextAusDateiAuslesProfi:
Danke, auf einen solchen Code wollte ich heraus, auch wenn er zur Zeit die Datei komplett einliest.

Gero_Programmierstil_de

Entweder du nimmst die full-C++ lösung von TextAusDateiAuslesProfi (konstruktivster beitrag als "nicht-member", heftig) un baust den crc dazu oder, wenns ähnlich bleiben soll, nimmst du den code, der anhängt.

OT:
Ist soooo krass, wie wenig problemorientiert hier teils geantwortet wird.
Hauptsache, alle Fehler vom Thread-Starters finden.
Naja, egal, da dies jetzt schon gemacht wurde, hier nur noch bisel code (untested,
lies die comments, da stehn noch paar anregunden un korrekturen von deinem code)

HANDLE hFile = CreateFile(szFilename, FILE_GENERIC_READ, FILE_SHARE_READ, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
if (hFile == INVALID_HANDLE_VALUE) // reicht
{
    dwErrorCode = GetLastError();
}
else
{
    DWORD dwFileSize = GetFileSize(hFile, NULL); // besser GetFileSizeEx()

    // Für die folgende Zeilen siehe GetFileSize() doku
    DWORD dwLastError = GetLastError();
    if ((dwFileSize == INVALID_FILE_SIZE) && (dwLastError != NO_ERROR))
    {
        dwErrorCode = dwLastError;
    }
    else
    {
        // Hier auf jeden fall noch checken, ob das file frößer als erwartet is. zb:
        // if (dwFileSize < MAXIMUM_FILE_SIZE)

        DWORD dwSetFilePointerRet = SetFilePointer(hFile, 10, NULL, FILE_BEGIN); // statt magic-value 10 besser konstante nehmen.

        // Für die folgende Zeilen siehe SetFilePointer() doku
        dwLastError = GetLastError();
        if ((dwSetFilePointerRet == INVALID_SET_FILE_POINTER) && (dwLastError != NO_ERROR))
        {
            dwErrorCode = dwLastError;
        }
        else
        {
            BYTE *buffer = (BYTE *)malloc(dwFileSize);
            DWORD dwBytesRead = 0;

            BOOL bSuccess = ReadFile(hFile, buffer, dwFileSize, &dwBytesRead, NULL); // entweder &buffer[0] oder buffer, aber nicht &buffer
            if ((bSuccess == FALSE) || (dwFileSize != dwBytesRead))
            {
                dwErrorCode = GetLastError();
            }
            else
            {
                dwCrc32 = CalculateBufferCRC(buffer, dwBytesRead);

                // your part: CRC checking, bla
                //.....
                //....
            }
            free(buffer);
        }
    }
}