STL map problem

Started by
10 comments, last by silvermace 15 years, 9 months ago
Hi! This is my first post here. (Edit: I donno whats the BBCode to show the Codes here) I've a Class called Var which handles multiple basic datatypes like int, float, char, string etc.. that Class works file when I use it like this.
Var x = (Var)2;
cout<<x;
But When I made a map of string and var its going Crazy.
typedef vector<string> 			stringList;
typedef map<string, Var> 		paramsMap;

int main(int argc, char *argv[]){
	paramsMap lst;
	lst["x"] = (Var)2;
	lst["y"] = (Var)5;
	
	for(paramsMap::iterator it=lst.begin();it != lst.end();it++){
		cout<<it->first<<" => "<<it->second<<endl;
	}

  return EXIT_SUCCESS;
}
The Above Should Output.
x => 2
x => 5
But whet its showing is.
x => y
y => 5
[Edited by - nlbs on July 21, 2008 9:58:37 AM]
Advertisement
Use your debugger to determine what happens in the call "<< it->second". This should tell you why an incorrect value is output.
I've tried a lot but Cant figure out.
However Is it showing wrongly or values are being stored wrongly in the map.
Here is my Code.

var.h
http://pastebin.com/m576e6998
var.cpp
http://pastebin.com/m57e02d88
It may also help if you could tell us what Var actually is (how it's implemented). Use lowercase to spell [code] instead of in this forum. You can also use [source], which formats it nicer.

EDIT: Oh, I see you posted your code. Your code has problems. You can't just take the result of std::string::c_str() and save it for later. Whenever the std::string is destroyed (in your case that would be right after you call c_str()), the result of c_str() is no longer valid. You need to make a copy of the content and manage it yourself, or simply make __data into a std::string.

The second I saw that 'char *' in your code, I knew it was going to cause trouble.

Whenever you use a pointer in C++, you must handle the ownership semantics for that pointer.

As it is written, your code is completely oblivious to ownership of the __data pointer. In particular, you assign to that pointer the return value of string::c_str, which is only guaranteed to remain present in memory for as long as the originating string exists (and since it's a temporary string, it disappears right after the assignment).

Either handle the corresponding memory correctly (allocate enough memory to store data, copy data, then delete the memory when you destroy the object, also implement the assignment and copy constructor to perform the exact same operation, and wrap everything in try-catch blocks to ensure exception-safety), or use std::string for internal storage, or use boost::any which does exactly what your class attempts to do in a more generic way.
I understood the problem that you all focused but donno how to solve it exactly.
__data = ::toString(val).c_str();
Here isnt the value of c_str() is stored in __data as a COPY ??
alocating and dealocating memory would be a lot of trouble. and I dont want to use any external librery like boost or others (dont think I dont like boost I just wanna make it only with STL indepedent of external dependancies)
char* __tmp = ::toString(val).c_str();__data = &__tmp;
will that code solves the Problem that you focused ??
Just make __data be of type std::string and simply do
__data = ::toString(val);


By the way, I am not sure it is safe to use identifiers that start with two underscores... I forgot the exact rules, but some things of that type are reserved for the compiler.

But what is the solution if I keep __data as char*

However As far I know _Caps or __Caps is reserved. But Its __smallcase But I donno why Its reserved and what will happen if I use it.
If you want to keep the data as a char *, you have to manage its ownership, as ToohrVyk said. This implies at least allocating and deallocating memory, making copies and providing an operator=.

The C++ Standard says (17.4.3.1.2): "Each name that contains a double underscore (_ _) or begins with an underscore followed by an uppercase letter is reserved to the implementation" and "Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace."
Quote:Original post by nlbs
But what is the solution if I keep __data as char*


It's pretty simple: all you have to do is manually manage the data that the pointer points to. The easy solution would have been if the pointer was always used to point to a literal, because then you wouldn't have to handle the memory at all. However, since you can manipulate strings decided on at runtime, you'll have to do more memory manipulation on your own. This means:

  • Every time you need to initialize the pointer (in all the constructors including the copy constructor) you need to determine the size of the initial string, create a memory buffer large enough to accomodate it, and initialize the pointer with that new buffer memory. Then, inside a try-catch block, you copy the memory from the initial string to the new buffer, and delete the memory in the catch-all block before re-throwing the exception to avoid memory leaks. Any other constructor work should also be placed in the try-block for the exact same reasons.

  • In the destructor, delete the memory buffer associated to the pointer.

  • Write a swap function which swaps the memory buffers of two instances of your class using std::swap. Use this swap function to implement an exception-safe version of operator= that copy-constructs a copy of its argument and swaps it with the current instance. You may elect to detect self-assignments, if you wish.

  • Do not, under any circumstances, modify your pointer in any other member function, and keep it private.


These simple steps will ensure that your memory usage is correct.

This topic is closed to new replies.

Advertisement