[solved]fwrite not writing entire buffer? C++

[MadPumpkin]

I'm writing an archiving tool in C++ and I'm having a little issues with the way that fwrite is writing out it's last buffer in my export function. For simplicity and shortness I haven't included anything but those two functions, if it seems as though more information is required I'll give it...

This import function appears to be serializing and saving all of the data from each file I tell it to into the AlexandiraArchive's *ArchiveLump (or p_arLump as it's typedef'd to) As upon displaying the data directly, all of it is displayed. But when I run the export function it exports the header successfully, and then It doesn't seem to be writing (roughly 58 characters from the end of each lumps unsigned char* mData member) to my output file.

Any help would be awesome and much appreciated. Suggestions even?

Import file

Code: Select all

void AlexandriaArchive::Import( const std::string filePath )
{
	FILE* file;
	file = fopen( filePath.c_str(), "rb" );

	if( file )
	{
		p_arLump temp = new ArchiveLump();
		temp = mLumpTable;
		while( temp->mNext != 0 )
			temp = temp->mNext;
		
		p_arLump lump = new ArchiveLump();

		fseek(file, 0L, SEEK_END);
		lump->mSize = ftell(file);
		fseek(file, 0L, SEEK_SET);

		lump->mData = new unsigned char[lump->mSize];
		lump->mName = filePath;

		fread( lump->mData, sizeof(unsigned char)*lump->mSize, 1, file );
		temp->mNext = lump;

		mHeader.mNumLumps++;

		fclose(file);

		/*while( entry = readdir( dir ) )
		{
			if( strcmp(entry->d_name, ".") != 0 && strcmp(entry->d_name, "..") != 0 )
			std::cout << entry->d_name << "\n";
		}
		closedir(dir);*/
	}
}

Export archive

Code: Select all

void AlexandriaArchive::Export( const std::string filePath ) const
{
	FILE* archive;
	archive = fopen( filePath.c_str(), "wb" );

	if( archive )
	{
		fwrite( &mHeader.mSignature, sizeof(char[6]), 1, archive );
		fwrite( &mHeader.mVersionMajor, sizeof( unsigned char ), 1, archive );
		fwrite( &mHeader.mVersionMinor, sizeof( unsigned char ), 1, archive );
		fwrite( &mHeader.mFlags, sizeof( unsigned short ), 1, archive );
		fwrite( &mHeader.mNumLumps, sizeof( unsigned char ), 1, archive );

		std::cout << "Header written\n";

		p_arLump temp = new ArchiveLump();
		temp = mLumpTable;

		while( temp->mNext != 0 )
		{
			temp = temp->mNext;

			size_t tmpLen = temp->mName.length();


/***               This appears to be where the problem occured               ***/

			fwrite( &tmpLen, sizeof(size_t), 1, archive );
			fwrite( temp->mName.c_str(), sizeof(char)*tmpLen, 1, archive );
			fwrite( "\0", sizeof(char), 1, archive );

			fwrite( &temp->mSize, sizeof(unsigned long), 1, archive );
			fwrite( &temp->mData, sizeof(unsigned char)*temp->mSize, 1, archive );

			std::cout << "Lump written\n";
		}

		fclose(archive);
	}
}

[Nokurn]

Code: Select all

fwrite( &temp->mData, sizeof(unsigned char)*temp->mSize, 1, archive );

If mData is an unsigned char*, you are writing the address of a pointer. I strongly doubt this is your intention. Do not take the address of temp->mData.

You're leaking memory in both functions:

Code: Select all

p_arLump temp = new ArchiveLump();
temp = mLumpTable;

This allocates and constructs a new instance of ArchiveLump, then immediately discards it without deleting it. Might I suggest a new structure for this loop (in the Export function)?

Code: Select all

for ( p_arLump lump = mLumpTable; lump; lump = lump->next )

This has the added safety of avoiding a null-pointer dereference if the head node is null (empty list), as you would encounter in the condition of your while loop.

Code: Select all

fwrite( temp->mName.c_str(), sizeof(char)*tmpLen, 1, archive );
fwrite( "\0", sizeof(char), 1, archive );

You can write this a little more succintly:

Code: Select all

fputs( temp->mName.c_str(), archive );

[MadPumpkin]

Heh thanks a lot, I hadn't even realized I was printing the address because of the trailing data being what I wanted it to be. Terrible mistake, thank you. I like your for loop suggestion as well. Thanks for your help.

EDIT: Your loop doesn't seem to be working in export. My program writes no lumps to the output file with this.

[Nokurn]

Heh thanks a lot, I hadn't even realized I was printing the address because of the trailing data being what I wanted it to be. Terrible mistake, thank you. I like your for loop suggestion as well. Thanks for your help.

EDIT: Your loop doesn't seem to be working in export. My program writes no lumps to the output file with this.

I missed this bit at the beginning of the loop:

Code: Select all

temp = temp->mNext;

You skip over the first node. I would still recommend checking to make sure you aren't dereferencing a null pointer before using it, even if you keep the while loop. If you're absolutely, 100% positive that it well never be null in production, use an assertion instead.