Problem with fstream/sstream

[buuko]

I'm making a tilemap-engine, and I have a code here which should return a valid filename ( number.map, for example 1.map ) for a function which makes me a random test map. This code of mine however seems to make that filename something like 1.mapÍýýýýÝÝÝÝÝÝãÃ8yúî. Also the "writer"-function adds those odd chars in the end of every line. I can't understand what's wrong with these.

char *mapManager::returnFileName(int number) {
std::string a;
std::stringstream ss;
ss << number << ".map";
a = ss.str();
char *b = new char[a.size()];
memcpy(b,a.c_str(),a.size());
return b;
}
void mapManager::createMap(int number) {
char temp[MAP_SIZE];
int temp2;
std:fstream mapFile;
mapFile.open(returnFileName(number));

for (int i=0;i<MAP_SIZE;i++) {
for (int a=0;a<MAP_SIZE;a++) {
temp2 = sf::Randomizer::Random(0, 1);
temp[a] = numberToMapChar(temp2);
}
mapFile << temp << "\n";
}
mapFile.close();


}

[Philip Nicoletti]

1) You do not NULL terminate your character stream
2) you have a "new" , but no delete ... this is a memory leak
3) Why don't you return a string instead ?

std::string mapManager::returnFileName(int number) 
{
   std::string a;
   std::stringstream ss;
   ss << number << ".map";
   rerturn ss.str();
}

//

mapFile.open(returnFileName(number).c_str());

[Paul McKenzie]

I'm making a tilemap-engine,

I think you should learn C++ properly with much easier programs. Your code has a lot of issues.

char *mapManager::returnFileName(int number) 
{
	std::string a;
	std::stringstream ss;
	ss << number << ".map";
	a = ss.str();
	char *b = new char[a.size()];
	memcpy(b,a.c_str(),a.size());
	return b;
}

Take the above code:

1) There is no need to return a char*. If you want to return a string where the contents of the string will not be modified, then return a std::string.

If you're returning a buffer that can be changed from outside the function, consider this option:

#include <vector>
typedef std::vector<char> CharArray;

CharArray mapManager::returnFileName(int number) 
{ 
  	std::string a;
	std::stringstream ss;
	ss << number << ".map";
	a = ss.str();
        CharArray  chArray(a.begin(), a.end());
        chArray.push_back(0);
        return chArray;
}

Back to the code you wrote:

2) Who is responsible for "deleting" the memory you returned? This "who gets to delete me?" type of programming need not be done in most C++ programs. Your program is vulnerable to memory leaks and/or memory corruption.

3) If the reason you're returning a char* is because a third-party function has char* as one of the arguments, that doesn't mean you must declare a char* and figure out how to initialize it and send it to the function.

What it means is that function wants a pointer to some character (usually representing the start of a char buffer). The std::string::c_str() function provides such a thing (if the function will not change the string contents), or the std::vector<char> option can be used.

The bottom line is that for what you really want to do, you have a steep learning curve in C++ programming. Otherwise, you'll have a program that is unstable and easily broken.

Regards,

Paul McKenzie

[buuko]

Thanks for replies!
The reason I was using char*: I somehow thought that the mapFile.open() needed a char*. I changed it to string and now it works. However still I have to do something about those odd chars inside my new .map-file.

I think you should learn C++ properly with much easier programs. Your code has a lot of issues.

I'm just so impatient. I'm trying to learn classes and SFML-library while doing this. It may seem stupid, but it's nicer to learn new things while actually doing/trying-to-do something with them.

[Philip Nicoletti]

[quote]
However still I have to do something about those odd chars inside my new .map-file.
[/code]

It is probably the same type of problem ... you do not NULL terminate
a character string.

[Paul McKenzie]

Thanks for replies!
The reason I was using char*: I somehow thought that the mapFile.open() needed a char*.

Which is one of the points I made in my previous post.

When a function has one of its parameters a T* (where T can be any type), that doesn't mean you declare a T* and attempt to pass it. This mistake is made by many newbie programmers to both C++ and C.

struct foo
{
   int x;
   int y;
};

void fill_struct(foo *pFoo)
{
    pFoo->x = 10;
    pFoo->y = 20;
}

So assume we want to have a local foo object and have the fiil_struct function called on this object. An experienced C++ programmer would call the function like this:

int main()
{
    foo p;
    fill_struct( &p );
}

No problem. However, a newbie who does not know C++ would try soimething similar to this:

int main()
{
    foo *p = new foo;
    fill_struct( p );
    //...
    delete p;
}

And the reason for the code above is because the inexperienced C++ programmer sees foo* as a parameter to fill_struct, and believes they must also declare a foo* and pass that instead. This code makes a call to the allocator (so as to create a foo object dynamically). This code wouldn't pass a code review -- you have the overhead of calling the allocator (new/delete) to create and destroy an object and is totally unnecessary.

All the fill_struct function wants is the address of a foo object, and it will fill it in. The first example where a foo is declared, and just passes the address of that local foo object -- this would be the correct approach.

Regards,

Paul McKenzie

[itsmeandnobodyelse]

However still I have to do something about those odd chars inside my new .map-file.

In order to add the null termination you need to allocate one additional char and copy a zero char to that additional byte:

char * b = new char[a.size()+1];
strcpy(b, a.c_str());
....
delete []b;

When you output pointer b at the .... with cout it now has a zero char at end (cause c_str() member function guarantees that) and that's why the streaming now was able to find the end of the string properly and doesn't output garbage chars from uninitialized memory.

Note, if you had followed the advices given you wouldn't have returned a pointer to char but a std::string and therefore you would have got a proper output as well.