String wird falsch aus Datei gelesen (Binär)

krümelkacker

Rookie86 schrieb:

class BinaryFile
{
	std::fstream m_file;

//...

	template<class T>
	void WriteValue(T value)
	{
		m_file.write((const char*)(&value), sizeof(T));
	}

Das halte ich für gefählich, da der Typ hier automatisch deduziert werden kann. Ich würde den Benutzer zwingen, den richtigen Typ anzugeben:

template<class T> struct id {typedef T type;};

:::

    template<class T>
    void WriteValue(typename id<T>::type value)
    ::::

Ein portables Dateiformat kommt dabei aber auch nicht heraus.

Rookie86 schrieb:

void WriteValue(std::string value)
	{
		WriteValue(value.size());
		m_file.write((const char*)(value.c_str()), value.size() * sizeof(char));
	}

Tja, was liefert denn value.size() zurück -- vom TYP her?

Rookie86 schrieb:

template<class T>
	void ReadValue(T& value)
	{
		char* temp = new char[sizeof(T)];
		m_file.read(temp, sizeof(T));
		value = (T)(*temp);
		delete temp;
	}

Das delete passt nicht zum new[]. Es geht auch komplett ohne new[]/delete[]. Lass von new/delete und ganzbesonders new[]/delete[] einfach die Finger.

Rookie86 schrieb:

void ReadValue(std::string& value)
	{
		unsigned int chars = 0;
		ReadValue(chars);

Aha. Wer sagt denn, dass "unsigned int" der richtige Typ hier ist, der auch von string::size zurückgegeben wird? Das kann hier nach hinten losgehen...

Rookie86 schrieb:

char* temp = new char[chars];
		m_file.read(temp, chars*sizeof(char));
		value = temp;
		delete temp;
	};

Schon wieder new[] mit falschen delete. Nimm doch einfach std::vector<char>. Das ist dann auch Ausnahme-sicher. Also, wenn dir zwischendurch eine Ausnahme um die Ohren fliegt, dann gibt std::vector<char> ordnungsgemäß den Speicher frei, wobei bei Dir ein Speicherleck entstehen würde. Es hat schon seinen Sinn, dass wir Resourcen (inklusive Speicher) "an benutzerdefinierte Objekte binden". Rohe Zeiger sind einfach dämlich. Die fühlen sich nicht für die Verwaltung des Speichers, auf den sie zeigen, verantwortlich. Von dawegen hast Du hier mit Ihnen die manuelle Speicherverwaltung an der Backe...

Braunstein

Wenn wir schon dabei sind

m_file.write((const char*)(value.c_str()), value.size() * sizeof(char));

Wozu dient der cast zu const char*? c_str() liefert das doch schon.

Rookie86

Habs jetzt umgeschrieben und bin endlich das new/delete-Zeugs losgeworden:

template<class T>

	void WriteValue(T value)
    {
        m_file.write(reinterpret_cast<const char*>(&value), sizeof(value));
    }

    void WriteValue(std::string value)
    {
		value.push_back('\0');
		WriteValue<size_t>(value.size());
        m_file.write(value.c_str(), value.size() * sizeof(char));
	}

    template<class T>
    void ReadValue(T value)
    {
  		m_file.read(reinterpret_cast<char*>(&value), sizeof(value));
    }

    void ReadValue(std::string value)
    {
        size_t chars = 0;
        ReadValue<size_t>(chars);
		m_file.read(reinterpret_cast<char*>(&value), chars*sizeof(char));
    }

#include <iostream>

#include "BinaryFile.h"

int main(int argc, char** argv)
{
	std::string tmpString = std::string("TestString");
	int i1 = 1;
	int i2 = 23;

	BinaryFile* bf = new BinaryFile("test.txt");
	bf->WriteValue<std::string>(tmpString);
	bf->WriteValue<int>(i1);
	bf->WriteValue<int>(i2);
	delete bf;	

	BinaryFile* bf2 = new BinaryFile("test.txt");
	bf->ReadValue<std::string>(tmpString);
	bf->ReadValue<int>(i1);
	bf->ReadValue<int>(i2);
	delete bf2;	

	std::cout << tmpString.c_str() << std::endl;
	std::cout << i1 << std::endl;
	std::cout << i2 << std::endl;
	return 0;
};

Wenn er durch läuft gibt er mir die richtigen Werte aus... so weit so gut.
Die Zugriffsverletzungen treten aber immer noch auf :(.

@krümelkacker:

value.size() ist vom Typ size_t. size_t ist aber nichts anderes als typedef unsigned int size_t. Trotzdem habe ich den Typ einmal auf size_t gewechselt.

template<class T> struct id {typedef T type;};

:::

    template<class T>
    void WriteValue(typename id<T>::type value)
    ::::

Wie das funktioniert verstehe ich noch nicht ganz (deshalb ist es noch nicht im Code), habe aber bei jedem Write/Read den Typ manuell angegeben als würde mich der Code schon dazu zwingen.

@Braunstein: Ja logisch... hab die unnötige Konvertierung entfernt.

SeppJ

void WriteValue(T value)
    {
        m_file.write(reinterpret_cast<const char*>(&value), sizeof(value));
    }

Ich übersetz das mal: Du schreibst an die Stelle deiner Variable value, welche eine lokale Variable in deiner Funktion ist, soviele chars wie der Wert dieser Variablen.

Wenn's schon mit so einem Hauer losgeht, lese ich lieber gar nicht erst weiter.

314159265358979

@SeppJ: Bis auf die Tatsache, dass er den value per const-ref nehmen könnte, sehe ich da nichts schlimmes. Bin ich blind?

SeppJ

314159265358979 schrieb:

@SeppJ: Bis auf die Tatsache, dass er den value per const-ref nehmen könnte, sehe ich da nichts schlimmes. Bin ich blind?

Kleiner Übersetzungsfehler: Er schreibt nicht so viele Bytes wie im Wert stehen, sondern er überschreibt den Wert einfach. Das zerschießt ihm dann zwar nicht den Stack, es macht einfach nur gar nichts. Und wenn die Funktion dann noch so aufgerufen wird:

WriteValue<size_t>(value.size());

Dann ist klar, dass da was total nicht stimmt.

Rookie86

@SeppJ:

Ich übersetz das mal: Du schreibst an die Stelle deiner Variable value, welche eine lokale Variable in deiner Funktion ist, soviele chars wie der Wert dieser Variablen.

Nicht ganz, er schreibt von der Position von value soviele Bytes wie value gross ist, bspw. bei einem char also 1 Byte in die Datei. Was ja nicht falsch ist

Sorry, aber im Moment check ichs einfach überhaupt nicht mehr... ist wohl ne kurze Pause nötig und dann muss ich den Code noch mal genau durchspielen bzw. debuggen.

Braunstein

void ReadValue(std::string value)
    {
        size_t chars = 0;
        ReadValue<size_t>(chars);
        m_file.read(reinterpret_cast<char*>(&value), chars*sizeof(char));
    }

Das hier geht gar nicht. Du kannst doch nicht einfach auf die Position deines Strings irgendwelche Daten schreiben. std::string ist doch kein Array.
Wie wärs denn damit.

std::string ReadValue()
    {
        size_t size = 0; // name size gefällt mir hier einfach besser
        ReadValue(size);
        std::vector<char> tmp(size);
        m_file.read(&tmp[0], size * sizeof(char)); // das sizeof(char) finde ich hier unnötig
        return std::string(tmp.begin(), tmp.end());
    }

PS Sepps Kritik kann ich hier nicht nachvollziehen. write schreibt schließlich in den Stream und nicht in die Variable.

SeppJ

Rookie86 schrieb:

Nicht ganz, er schreibt von der Position von value soviele Bytes wie value gross ist, bspw. bei einem char also 1 Byte in die Datei. Was ja nicht falsch ist

Oh, ähh. Ich brauch 'nen Kaffee. Aber schnell...

Rookie86

@Braunstein: Es funktioniert!

Boah, war das ein Kampf... wieder was gelernt.

Vielen dank für eure Hilfe