C++ map object not growing when members added

Question

Below the map 'widgets' is always size of 1 for some reason. There should be 4 when it's done.

Output:

Widget: widget_ram_label:layout_bar:0 1
Widget: widget_ram_active:layout_bar:0 1
Widget: widget_ram_total:layout_bar:0 1
Widget: widget_wlan0_label:layout_bar:0 1

Here's widgets:

std::map<const char *, Widget *> widgets;

And here's the code:

void Generic::BuildLayouts() {
   for(std::map<const char*, std::vector<widget_template> >::iterator l = 
       widget_templates.begin(); l != widget_templates.end(); l++) {
       std::vector<widget_template> layout = l->second;
       for(unsigned int i = 0; i < layout.size(); i++ ) {
           Json::Value *widget_v = CFG_Fetch_Raw(root, layout[i].key);
           if(!widget_v) {
               error("No widget named <%s>", layout[i].key);
               continue;
           }
           Json::Value *type = CFG_Fetch_Raw(widget_v, "type");
           if(!type) {
               error("Widget <%s> has no type!", layout[i].key);
               delete widget_v;
               continue;
           }

           Widget *widget;

           char name[256];
           sprintf(name, "%s:%s", layout[i].key, l->first);
           char tmp[256];
           int i = 0;
           sprintf(tmp, "%s:%d", name, i);

           while(widgets.find(tmp) != widgets.end()) {
               i++;
               sprintf(tmp, "%s:%d", name, i);
           }
           memcpy(name, tmp, 256);

           if(strcmp(type->asCString(), "text") == 0) {
               widget = (Widget *) new WidgetText(this, name, widget_v, 
                   layout[i].row, layout[i].col);
               std::cout << "Widget: " << name << " " << widgets.size() << std::endl;
           } else {
               error("Unknown widget type: %s", type->asCString());
           }
           if(widget) {
               widgets[name] = widget;
           }
           delete type;

       }
   }
}

Answer 1

Maybe because all your name pointers poitns to the same buffer? Because the content of name changes, but not the value of the pointer to name in the map.

Try to use std::string instead.

Replace you name buffer by

#include <string >
//...
std::string name = "the name";

and replace your map by

std::map< const std::string , Widget* > widgets;

That will make your life easier, safer and more readable.

To help formatting, use std::stringstream or boost string algorithms.

---

A start of example, this code :

                            char name[256];
   sprintf(name, "%s:%s", layout[i].key, l->first);
   char tmp[256];
   int i = 0;
   sprintf(tmp, "%s:%d", name, i);

   while(widgets.find(tmp) != widgets.end()) {
    i++;
    sprintf(tmp, "%s:%d", name, i);
   }
   memcpy(name, tmp, 256);

would be written like this:

Widget *widget = NULL; // always initialize your variables!!!

std::stringstream name_stream; // this will let us format our string
       name_stream << layout[i].key << ":" << l->first;

std::string name = name_stream.str(); // now we got the string formated.

std::stringstream tmp_stream; // same for tmp
tmp_stream << name << ":" << i; // will automatically convert basic types, see the doc if you want specific formatting

std::string tmp = tmp_stream.str(); // now we got the string formated.

// the while loop have no sense : a map have only one value by key
// if you want to have several values by key, use std::multi_map instead -- it don't work exactly the same though
// for now i'll just assume you just need to find the value associated to the name:


typedef std::map< const std::string, Widget* > WidgetMap; // for ease of writing, make a shortcut! ease your life!
WidgetMap::iterator tmp_it = widgets.find( tmp );

if( tmp_it != widgets.end() ) 
{   
// starting here I don't understand you code, so I'll let you find the rest :)
}

Answer 2

It's probably hashing on the address of name. Use std:string.

Answer 3

As an alternative to using std::string, you can just strdup the buffer before using it. Just remember to free it when you free your std::map.

Answer 4

std::map<const char *, Widget *> widgets;

Don't do this. Never use char* as a map key. (The map uses the std comparison std::less for its keys and that compares the addresses for pointers.)

Do yourself a favour and use std::string. Once you mastered this, you might try to go back dealing with C strings again.

Answer 5

Your map doesn't know how to compare C-style strings properly. If you insist on using C-style strings as keys for your map, you need to supply the proper comparator. For example, it could be done as follows

inline bool str_less(const char *s1, const char *s2) {
  return strcmp(s1, s2) < 0;
}

...
std::map<const char *, Widget *, bool (*)(const char *, const char *)> widgets(str_less);

Or, if you wish, you can implement the comparator as a class-based functor.

But that's not all. Yor map does not know how to and will not manage memory for your 'const char *' keys. You are passing a pointer to a local buffer to the map as a key. This is totaly useless. In case of 'const char *' keys you are supposed to manage the key memory by yourself. For example, you can allocate each key dynamically and pass a pointer to such a dynamically allocated and properly initialized key buffer to the map. And later, when the time comes to destroy your map, it is your responsiblility to deallocate all these key buffers manually.

This all is a major pain in the neck. For this reason you should be much better off using 'std::string' as the key type, not a C-style string.