3. ein absolut "kranker" fehler

ein absolut "kranker" fehler

  • O

    read funktion:

    result Virtualfile::read(char* &buffer,unsigned int bytes){
    if(datapointer!=NULL){
        if(position+bytes<size){
            for(unsigned int i=0;i<bytes;++i){
                buffer[i]=readpointer[position+i];
            }
            position+=bytes;
            return ok;
        }
        return out_of_range;
    }
    return stream_not_open;
}

    open funktion

    result Virtualfile::open(char* Name){
    if(datapointer==NULL){
        ifstream Data(Name);
        if(Data.is_open()){
            Data.seekg(0,ios::end);
            size=Data.tellg();
            Data.seekg(0,ios::beg);
            datapointer=malloc(size);
            if(datapointer!=NULL){
                readpointer=(char*)(datapointer);
                position=0;
                name=Name;
                char Buffer;
                for(unsigned int i=0;i<size;++i){
                    Data.read(&Buffer,sizeof(char));
                    readpointer[i]=Buffer;
                }
                Data.close();
                return ok;
            }
            size=0;
            Data.close();
            return failed;
        }
        return wrong_arguments;
    }
    return stream_allready_open;
}
    0
  • S
     

    //result? benutz lieber exceptions
result Virtualfile::read(char* &buffer,unsigned int bytes){
    if(datapointer!=NULL){ //wozu? einfach den ctor die oeffnen
//ueberlassen, dann brauchst du den kaese nicht
//und selbst wenn, dann mach doch bitte
//if(!datapointer) return stream_not_open;
//bzw. natuerlich exception.
        if(position+bytes<size){
            for(unsigned int i=0;i<bytes;++i){
                buffer[i]=readpointer[position+i]; //ich sehe kein malloc/new
            }
            position+=bytes;
            return ok;
        }
        return out_of_range;
    }
    return stream_not_open;
}

    sehe ich das richtig, dass du alles im speicher haeltst? was ist, wenn die datei n paar 100 MB gross ist?

    result Virtualfile::open(char* Name){ //const vergessen
//wobei ich sowieso lieber den ctor nehmen wuerde
//man oeffnet naemlich keine datei mehr als 1 mal
    if(datapointer==NULL){
        ifstream Data(Name);
        if(Data.is_open()){
            Data.seekg(0,ios::end);
            size=Data.tellg();
            Data.seekg(0,ios::beg);
            datapointer=malloc(size);
            if(datapointer!=NULL){
                readpointer=(char*)(datapointer);
                position=0;
                name=Name;
                char Buffer;
                for(unsigned int i=0;i<size;++i){
                    Data.read(&Buffer,sizeof(char));
//sehr lahm
                    readpointer[i]=Buffer;
                }
                Data.close(); //unnoetig
                return ok;
            }
            size=0;
            Data.close(); //unnoetig
            return failed;
        }
        return wrong_arguments;
    }
    return stream_allready_open;
}

    und wieder keine exception...

    0
  • O

    ja ich halte alles im speicher, ja ich weis, dass es sehr viel werden kann, nein ich weis nicht, wie groß der heap ist,aber hab mal gelesen, dass alles was nicht stack ist für den heap reserviert ist.

    das system soll für texturen modeldaten/animationsdaten usw sein,also alles was man mehrere male in kurzen abständen brauchen kann, und wo sich das ablegen im meory lohnt,ich plane ja auch schon ne 2. klasse die direkt aus der datei streamen kann(ich will die dateien aus großen archiven laden,deshalb sind die standard streamklassen nich so perfekt^^).

    der ram is im gegensatz zur hd von der geschwindigkeit her einfach zu verlockend^^

    so zu deinen kommentaren noch:
    exceptions hab ich bisher noch nie benutzt, ist für mich also relatives neuland 😃

    beim rest kann ich dir soweit zustimmen, bis auf die sache mit malloc/new.. das problem ist, dass ich nicht überprüfen kann,ob schon speicher reserviert worden ist, und ich kann auch nicht sicher gehen, dass der speicher der von der read funktion reserviert wird auch wieder freigegeben wird.Die daten die rausgegeben werden können ja auch programmtechnisch ne längere lebensdauer als die klasse haben.
    desweiteren wirds in den meisten fällen nicht nötig sein,speicher extra zu reservieren..(datei.read(Header,sizeof(header)//hier ist speicher reservieren unnötig, er existiert ja schon^^).

    und zu der sache mit ctor..ka was das ist, hab auf die shcnelle weder was in der bcb hilfe noch in meinem nachschlagewerk gefunden^^

    //sehr lahm

    so besser?

    Data.read(readpointer,size);
    0
  • M

    ctor = Konstruktor und wenn du den nicht kennst, solltest du C++ mal richtig lernen.
    Im Übrigen ist dies das Standard C++ Forum, weswegen man dir kaum etwas außerstandardmäßiges für so ein Problem empfehlen wird, die BCB Doku ist daher nicht einzusehen.

    0
  • O

    was ein konstruktor ist weis ich,ich kenn nur nicht die fachbegriffe.
    und mein konstruktor macht das ja auch, aber ich hab ja in der Klasse noch die möglichkeit gelassen,die zieldatei zu ändern,also dass man zb im konstruktor eine virtuelle datei öffnet, danach mit ihr arbeitet, sie am ende wieder schließt, und dann wieder eine neue öffnet.

    0
  • M

    Konstruktor ist der Fachbegriff (und der einzige), ctor ist eine Abkürzung ;)...

    Ansonsten machen die filestreams das doch auch, dass man neu öffnen kann, bei Bedarf, oder was meinst du jetzt, Shade? 😕

    0
  • O

    so, es funktioniert jetzt, es lag an der zeile:if(position+range<size){ hätte <= sein müssen^^

    hab auch grad mal den geschwindigkeits unterschied zwischen normalem read und meinem read getestet...beide sind 10millionen mal durchgelaufen..
    als zieldatei musste eine 33kb gro0e textdatei herhalten
    standard stream 2223
    mein stream 140(!)
    noch deutlicher wirds beim seek,da hab ich zuerst an der beg.pos dann end.pos dann cur.pos gemessen
    standard stream:8652|27250|11570
    mein stream:161|141|140

    diese werte überaschten mich jetzt doch ziemlich,deshalb hab ich nochmal alles nachgeprüft,ob ich nich irgendwo messfehler drin hatte, aber seht selbst:

    vfile::Virtualfile file;
    file.open("b.txt");
    int k=GetTickCount();
    for(int i=0;i<10000000;++i){
        file.seek(0,vfile::end_pos);
    }
    cout<<GetTickCount()-k<<endl;
    file.close();

    ifstream datei("b.txt");
    int l=GetTickCount();
    for(int i=0;i<10000000;++i){
        datei.seekg(0,ios::end);
    }
    cout<<GetTickCount()-l;

    also ich denke, der ärger hat sich gelohnt,oder was meint ihr? 😉

    0
  • S

    @Mis2Com:
    jo, std::fstream hat auch ne open methode und close. Aber die streams sind ja auch mies designt. Der Designer davon (hab leider den namen vergessen) gibt es ja auch zu und hat gemeint, dass er einiges anders machen wuerde, wenn er sie neu designen koennte.

    @otze:
    natuerlich ist es schneller alles zuerst in den speicher zu laden.
    die streams sind ja auch nicht fuer das schnelle auslesen aus einer datei gemacht - sie puffern ja wie wild 😉

    btw: was misst du denn da? nur das seek alleine? und das ganze auslesen dass du im open machst, ignorieren wir schoen... nicht sehr representativ...

    und bitte: verwende exception und den ctor und dtor statt open und close. wenn du nicht weisst warum, google mal nach RAII

    nix fuer ungut, aber deine klasse ist nicht so toll.
    zB bei einem 100MB file stehst du an.

    ausserdem allokiert deine read methode keinen speicher - somit ist dein urspruenglicher code (mit dem ach so mysterioesen fehler) fehlerhaft.

    0
  • O

    shade? ich glaub wir reden aneinander vorbei 😃

    die read methode darf garkeinen speicher allokieren, das würde zu gigantischen speicherleaks führen.
    wenn ne datei geöffnet wird, wird entweder sofort oder mittels open ein stream auf die zieldatei geöffnet, sie wird vermessen, dann mit dem wert der speicher reserviert, und die datei reingeschrieben.
    wenn die klasse den scope verlässt wird der platz wieder freigegeben.
    die read methode soll dann nur angeforderte mengen speicher in einen anderen speicherbereich transferieren.
    read darf nicht new/malloc benutzen,was man auch an folgendem code sehen kann:

    header Header;
datei.open("file.txt");
datei.read((*void)(&Header),sizeof(header));//hier ein new und wir haben ein schönes speicherleck,der cast muss da sein,
//da ich keine templates benutzen kann,und deshalb eine version für void zeiger machen musste^^
//anderes Beispiel
char data[256];
datei.read(data,256);//speicherleck bei new
char* subdata=new char[256];
datei.read(subdata,256);//keiner erklärung mehr von nöten

    zum thema open und ausmessen...seek wird andauernd durchgeführt read auch,open dagegen fast garnicht, also wieso testen? ist uninteressant,was nu länger braucht.
    Und 100mb Files sind ne schöne sache, die werden nur partweise ausgelesen,oder direkt mittels stream,daran hab ich schon gedacht.
    andererseits haben zb modeldaten die angewohnheit, dass man da wild rumseeken muss,und da hat so ein memorystream einen echten heimvorteil 😉

    0
  • M

    result Virtualfile::read(char* &buffer,unsigned int bytes){
    if(datapointer!=NULL){
    if(position+bytes<size){
    for(unsigned int i=0;i<bytes;++i){
    buffer*=readpointer[position+i]; // KUCKUCK
    }
    position+=bytes;
    return ok;
    }
    return out_of_range;
    }
    return stream_not_open;*

    Ob ich C++ kann oder nicht, ist mir jetzt egal, Fakt ist, dass du in der Zeile [i]KUCKUCK* diesen Buffer, den man übergibt, beschreibst und dafür dass du ihn beschreibst, muss Speicher vorhanden sein, den du nicht allokiert hast.
    Und es handelt sich auch nicht um irgendeinen Stream, auf den du schreibst, du schreibst in den Buffer und der ist nicht definiert, da du bisher einfach einen Zeiger angelegt hast, der irgendwo hin zeigt.

    Jedenfalls sehe ich das so:

    void irgendwas(char*& bla)
{
for(int n = 0; n < 100; ++n)
bla[i] = 'a'; // irgendwas
}

char* a;
irgendwas(a);

    Somit schreibst du irgendwo in den Speicher, ob es da hingehört oder nicht.
    Das Ergebnis sollte undefiniert sein, oder nicht?
    In diesem Fall MUSS Speicher allokiert werden und das ist nicht im Gegenteil ein Speicherleck, es sei denn, du vergisst delete natürlich.

    Nagut, offensichtlich bin ich blind, man kläre mich auf...

    0
  • O

    wenn ich ausserhalb der funktion speicherplatz für ein objekt reserviere, und innerhalb der funktion dann mit einer referenz auf dieses objekt(bzw dem zeiger auf den speicherplatz den das objekt reserviert hat)arbeite,dann brauch ich nicht nochmal speicherplatz reservieren, denn er ist ja schon reserviert.
    und nun stellen wir uns mal vor, ich reserviere ausserhalb der funktion speicher, übergebe den zeiger auf diesen speicher, und versuche dann mittels new nochmal speicher für diesen pointer zu reservieren.
    ich kann nur vermuten was passieren kann:
    1. undefiniertes verhalten
    2. der pointer zeigt auf den gleichen bereich
    3. der pointer wird einem neuen bereich zugeordnet, dabei entsteht ein speicherleck, da der vorher reservierte speicher nichmehr deleted wird.
    //edit nach selbstversuch: 3 trifft zu

    es besteht natürlich wie man in meinem ersten veralteten beispiel sehen kann die gefahr, illegal speicher zu belegen,aber solange mir niemand sagen kann,wie man testen kann, ob speicher reserviert wurde, bleibt das so.

    0
  • M

    Dein read-Methode stellt in keinem Fall Speicher für buffer bereit, wenn doch, so zeige mir die Zeile.

    0
  • O

    hallo? liest du überhaupt was ich schreibe? ich will garkeinen speicher zur verfügung stellen.
    ich will als funktionsparameter einen zeiger auf einen speicherbereich,den der programmierer in irgendeiner weise schon freigegeben hat.
    seis new, oder einfache reservierung des speichers auf dem stack, das is mir egal.
    es ist also die schuld des benutzers der read funktion, wenn er undefiniertes verhalten erhält,das hab ich aber auch schon irgendwo geschrieben bzw eingeräumt, da ich ja am amfang denselben fehler gemacht hab(auch wenn der eigentliche fehler woanders lag).

    0
  • M

    Selber hallo.

    Du schreibst in deiner Funktion in buffer rein und zwar recht viel.
    Und dieser Speicher ist NIRGENDWO allokiert, in deinem Beispiel nicht außerhalb und innerhalb der read-Funktion auch nicht, es ist mir völlig egal, was du willst, du kannst unallokierten Speicher nicht einfach vollschreiben.
    Wenn du innerhalb der read-Funktion keinen Speicher bereitstellst, so musst du dies außerhalb tun, aber das tust du nicht, folglich schreibst du in nicht-allokierten Speicher.

    Seufz, was ist daran so schwer zu verstehen?

    0
  • O

    ok nochmal ganz ganz langsam extra für dich-.-

    //dies ist jetzt ein erklärendes beispiel
char bla[254]
//es werden 254 bytes auf dem stack reserviert,sagen wir von der
//datenposition 16 aus,der reserviertespeicher geht also von 16-259
cout<<bla//hier wird die datenposition also 16 ausgegeben
//so irgendwo später im code nachdem eine datei geöffnet wurde
data.read(bla,254);
//hier wird der zeiger auf die position 16 und die info über die anzahl der zu
//kopierenden daten(zufälligerweise die größe des vorher reservierten speichers)übergeben
//innerhalb der funktion
result Virtualfile::read(char* &buffer,unsigned int bytes){ 
//durch die übergabe des zeigers auf den speicherbereich 16 zeigt buffer nun auch auf 16(siehe funktionen und parameter)
buffer[1]//da der buffer den wert 16 hat zeigt buffer[1] 1 bit weiter->17->innerhalb des reservierten bereichs also ok
buffer=readpointer[position+i]//zulässig solange es im reservierten bereich bleibt
//das selbe gilt für readpointer->malloc
//usw usf

    .

    0
  • M

    Ich bezog mich auf dieses Beispiel:

    result Virtualfile::read(char* &buffer,unsigned int bytes){
    if(datapointer!=NULL){
        if(position+bytes<size){
            for(unsigned int i=0;i<bytes;++i){
                buffer[i]=readpointer[position+i];
            }
            position+=bytes;
            return ok;
        }
        return out_of_range;
    }
    return stream_not_open;
}

    Außerdem klang es so, als wenn ein new außerhalb der Funktion ein Speicherleck produzieren würde, aber lass mal, ich versteh ja sowieso alles falsch. 👎

    0
  • S

    otze schrieb:

    die read methode darf garkeinen speicher allokieren, das würde zu gigantischen speicherleaks führen.

    Exakt.
    Beachte aber mal dein ursprüngliches Beispiel:

    int main(int argc, char* argv[])
{
    int blub;//ganz doll wundern^^
    int i;
    vfile::Virtualfile file;
    file.open("3DSINFO.TXT");//datei in speicher einlesen
    char* data;
    file.read(data,file.get_size());//inhalt des speichers in variable
    for(unsigned int i=0;i<file.get_size();++i){
        cout<<data[i];//ausgabe der datei,bei long double in der blub variable gibts access violation in module bla^^
    }
    file.close();
    cin>>i;
    return 0;
}

    data hat keinen Speicher...

    Warum nimmt read() denn dann eigentlich den Zeiger by Reference?

    wenn ne datei geöffnet wird, wird entweder sofort oder mittels open

    Und genau das ist der Designfehler. Lass das open weg.

    zum thema open und ausmessen...seek wird andauernd durchgeführt read auch,open dagegen fast garnicht, also wieso testen? ist uninteressant,was nu länger braucht.

    seek? Ich seeke Quasi nie, denn seek ist lahm. Wozu sollte ich das tun?
    Wenn ich seeken will, mappe ich den Inhalt der Datei in den RAM und operiere nur auf ihm -> ohne kopien.

    andererseits haben zb modeldaten die angewohnheit, dass man da wild rumseeken muss,und da hat so ein memorystream einen echten heimvorteil 😉

    Dann mach das ganze ohne Kopien. Du willst im Prinzip nur rum springen in der Datei -> wozu also Kopien erstellen?

    Ich habe noch nie mit Models gearbeitet, aber ich würde es einfach in den RAM mappen und mir nur Zeiger auf die Interessanten stellen merken - uU lieber zuerst in ein parserfreundlicheres Format kompilieren.

    0
  • O

    sorry, dass ich etwas azsgetickt bin,ich versuch jetzt nochmal in ruhe alles zu klären.

    also,wenn ich in einen externen buffer schreiben will hab ich ein grundsätzliches problem:ich weis nicht,ob genügend speicher bzw ob überhaupt speicher freigegeben wurde.
    nun gibt es 2 möglichkeiten:
    1.ich allokiere immer innerhalb der funktion speicher.
    da gibt es 2 probleme:
    a) der user kann nur mit pointern arbeiten,bzw müsste den von mir allokierten speicherplatz in ein normales objekt kopieren.
    b)wenn der user schon im vorfeld speicher allokiert hat, geht der natürlich verloren, und auf den stack kann man halt nur durch kopieren aus dem heap sachen ablegen
    2.ich allokiere nur ausserhalb der funktion speicher
    und wieder 2 probleme:
    a) vergisst der user speicher zu allokieren lande ich im undefinierten bereich
    b) der user muss immer genügend speicher im vorfeld freigeben
    3.ich allokiere nur bei bedarf
    1 grundsätzliches problem:
    man kann nur schlecht die menge von mit new allokiertem speicher rausbekommen, bei normalen arrays funktioniert sizeof,ich müsste also immer 2 überprüfungen bei jedem read vorgang durchführen..

    ich hab mich also für punkt 2 entschieden,und überlasse dem user die ganze arbeit.
    new ausserhalb der funktion wirft nur ein speicherleck auf, wenn ich mich für punkt 1 entscheiden würde.

    @shade den fehler am anfang hab ich jetzt hier in diesem thread 3 mal zugegeben, wieso merkt das eigentlich keiner und reitet weiter drauf rum?

    zeiger bei reference? hast eigentlich recht, ich weis nur nicht genau wiesehr sich der overhead beim erstellen und zerstören der zeiger ist...

    seek brauch ich vorallem dann,wenn ich die dateien in ein handliches format bringen will(zb wenn ich von d3ds zu einem eigenen format konverte und danach wieder abspeicher),ohne das mappen in den memory dauert die ganze prozedur einfach zu lange.
    ausserdem ist mein seek wirklich schnell im gegensatz zu dem der streams 😃

    aber nochmal eine grundsätzliche frage zu der mir ans herz gelegten ausnahmebehandlung 😉
    wie soll ich auf den fehler reagieren, dass der user einen unbekannten namen eingegeben hat, bzw mittem im programm eine nicht existente datei aufgerufen wird?
    oder wnen malloc den nicht genug memory fehler ausgibt(was doch eigentlich sehr unwahrscheinlich sein sollte oder?)

    0
  • M

    gr, also das ist mir jetzt nun auch klar, das ist genau so wie beim normalen read, ich hatte mich verdammt nochmal nur auf den falschen Code bezogen...

    0
  • S

    otze schrieb:

    2.ich allokiere nur ausserhalb der funktion speicher
    und wieder 2 probleme:
    a) vergisst der user speicher zu allokieren lande ich im undefinierten bereich
    b) der user muss immer genügend speicher im vorfeld freigeben

    Dies ist die richtige Methode.
    Der User übergibt ja als 2. Parameter die anzahl der Bytes die du lesen sollst 🙂
    Also weiss nur er wieviel speicher er braucht und kann das optimal einteilen.
    wenn der user zu dumm ist, speicher zu allokieren, ist das nicht deine schuld. detto beim freigeben des speichers.

    zeiger bei reference? hast eigentlich recht, ich weis nur nicht genau wiesehr sich der overhead beim erstellen und zerstören der zeiger ist...

    Kaum overhead. Aber es verwirrt. Mich hat es ziemlich verwirrt.

    ausserdem ist mein seek wirklich schnell im gegensatz zu dem der streams 😃

    Logisch. Denn die sind für lesen und schreiben gemacht - deins ist nur für lesen und fürs seeken.

    wie soll ich auf den fehler reagieren, dass der user einen unbekannten namen eingegeben hat, bzw mittem im programm eine nicht existente datei aufgerufen wird?

    du wirfst eine Exception, zB file_not_found() oder ähnliches.
    uU solltest du noch eine Funktion anbieten wo der user testen kann, ob es die datei gibt.

    um solchen code zu ermöglichen.

    vfile* f;
if(file_exists("foobar"))
{
  f=new vfile("foobar");
}
else
{
  f=new vfile("other");
}

    ein is_open wie die C++ streams ist doof. Denn wir wollen mit exceptions die Fehlerbehandlung nicht vorort vornehmen, sondern dort, wo wir darauf reagieren können.

    oder wnen malloc den nicht genug memory fehler ausgibt(was doch eigentlich sehr unwahrscheinlich sein sollte oder?)

    einfach bad_alloc weiter werfen

    0
Anmelden zum Antworten