-2

I want to keep an array of data index by a string and I thought I'd best use std::map for this purpose. I have below example code:

 typedef struct MyType_s {
     long long timestamp;
     int cnt;
     bool parked;
 }MyType;

 static MyType *list = {0};
 static int listcnt  = 0;

 //-------------------------------------------------------------------------------------------------

 int map_add_item(std::map<std::string, MyType*> *pmap, std::string str, long long tmestmp)
 {
     if (listcnt == 0){
         list =(MyType*)malloc(sizeof(MyType));
         if (list)
             listcnt++;
         else
             return ENOMEM;
     }
     if (realloc(list,sizeof(MyType)*(++listcnt))==0)
         return ENOMEM;
     list->timestamp = tmestmp;
     if (!(str.length()&&tmestmp&&pmap))
         return EINVAL*-1;
     if (pmap->insert(std::make_pair(str, &list[listcnt-1])).second == false){
         pmap->find(str)->second->timestamp = tmestmp;
         return EEXIST*-1;
     }

     return OK;
 }

which compiles fine but I get a mem dump like this when I run it:

*** Error in `./std_map': double free or corruption (fasttop): 0x000000000226ec20 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7f19fb6267e5]
/lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7f19fb62f37a]
/lib/x86_64-linux-gnu/libc.so.6(+0x83350)[0x7f19fb632350]
/lib/x86_64-linux-gnu/libc.so.6(realloc+0x179)[0x7f19fb633839]
./std_map[0x4013c8]
./std_map[0x40198d]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf0)[0x7f19fb5cf830]
./std_map[0x401259]
======= Memory map: ========
00400000-00404000 r-xp 00000000 08:02 6054199                            /path/to/src/tmp/std_map
00604000-00605000 r--p 00004000 08:02 6054199                          /path/to/src/tmp/std_map
00605000-00606000 rw-p 00005000 08:02 6054199                            /path/to/src/tmp/std_map
0225d000-0228f000 rw-p 00000000 00:00 0                                  [heap]
7f19f4000000-7f19f4021000 rw-p 00000000 00:00 0 
7f19f4021000-7f19f8000000 ---p 00000000 00:00 0 
7f19fb2a6000-7f19fb3ae000 r-xp 00000000 08:02 28971840                   /lib/x86_64-linux-gnu/libm-2.23.so
7f19fb3ae000-7f19fb5ad000 ---p 00108000 08:02 28971840                   /lib/x86_64-linux-gnu/libm-2.23.so
7f19fb5ad000-7f19fb5ae000 r--p 00107000 08:02 28971840                   /lib/x86_64-linux-gnu/libm-2.23.so
7f19fb5ae000-7f19fb5af000 rw-p 00108000 08:02 28971840                   /lib/x86_64-linux-gnu/libm-2.23.so
7f19fb5af000-7f19fb76f000 r-xp 00000000 08:02 28971844                   /lib/x86_64-linux-gnu/libc-2.23.so
7f19fb76f000-7f19fb96f000 ---p 001c0000 08:02 28971844                   /lib/x86_64-linux-gnu/libc-2.23.so
7f19fb96f000-7f19fb973000 r--p 001c0000 08:02 28971844                   /lib/x86_64-linux-gnu/libc-2.23.so
7f19fb973000-7f19fb975000 rw-p 001c4000 08:02 28971844                   /lib/x86_64-linux-gnu/libc-2.23.so
7f19fb975000-7f19fb979000 rw-p 00000000 00:00 0 
7f19fb979000-7f19fb98f000 r-xp 00000000 08:02 28971397                   /lib/x86_64-linux-gnu/libgcc_s.so.1
7f19fb98f000-7f19fbb8e000 ---p 00016000 08:02 28971397                   /lib/x86_64-linux-gnu/libgcc_s.so.1
7f19fbb8e000-7f19fbb8f000 rw-p 00015000 08:02 28971397                   /lib/x86_64-linux-gnu/libgcc_s.so.1
7f19fbb8f000-7f19fbd01000 r-xp 00000000 08:02 23072621                   /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21
7f19fbd01000-7f19fbf01000 ---p 00172000 08:02 23072621                   /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21
7f19fbf01000-7f19fbf0b000 r--p 00172000 08:02 23072621                   /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21
7f19fbf0b000-7f19fbf0d000 rw-p 0017c000 08:02 23072621                   /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.21
7f19fbf0d000-7f19fbf11000 rw-p 00000000 00:00 0
7f19fbf11000-7f19fbf37000 r-xp 00000000 08:02 28971842                   /lib/x86_64-linux-gnu/ld-2.23.so
7f19fc0fc000-7f19fc102000 rw-p 00000000 00:00 0
7f19fc135000-7f19fc136000 rw-p 00000000 00:00 0
7f19fc136000-7f19fc137000 r--p 00025000 08:02 28971842                   /lib/x86_64-linux-gnu/ld-2.23.so
7f19fc137000-7f19fc138000 rw-p 00026000 08:02 28971842                   /lib/x86_64-linux-gnu/ld-2.23.so
7f19fc138000-7f19fc139000 rw-p 00000000 00:00 0
7ffd68bf4000-7ffd68c16000 rw-p 00000000 00:00 0                          [stack]
7ffd68d88000-7ffd68d8b000 r--p 00000000 00:00 0                          [vvar]
7ffd68d8b000-7ffd68d8d000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]

Command terminated

gdb doesn't give me much more information too. This is after I call the function like this from main():

 int main()
 {
     int ret = 0;
     std::map<std::string, MyType*> mapOfWords;
     std::cout << map_add_item(&mapOfWords,"earth",time(NULL)+1) << std::endl;
     return 0;
 }

I'm wondering what I'm doing wrong, double free or corruption obviously is a hint but I'm not sure how to interpret this and I can't pinpoint the problem....

stdcerr
  • 13,725
  • 25
  • 71
  • 128
  • 4
    What is the reason you are doing this strange mix of C and C++ instead of writing proper C++ code? – UnholySheep Jun 12 '18 at 20:16
  • 4
    I'd guess realloc is doing what realloc is supposed to do and reallocating memory thereby invalidating the pointers you've stored in your map – Alan Birtles Jun 12 '18 at 20:25
  • 1
    Recommend leaving out line numbers. Should solving your problem require compiling your code, removing the line numbers can be a tedious chore. – user4581301 Jun 12 '18 at 20:28
  • @AlanBirtles The man page reads: *The realloc() function changes the size of the memory block pointed to by ptr to size bytes. The contents will be unchanged in the range from the start of the region up to the minimum of the old and new sizes. If the new size is larger than the old size, the added memory will not be initialized.* - the old data should not be invalidated... – stdcerr Jun 12 '18 at 22:25
  • @UnholySheep I would appreciate sugestions on how to improve my approach.... – stdcerr Jun 12 '18 at 22:26
  • 2
    From the same man page: `If the area pointed to was moved, a free(ptr) is done.` So realloc does have the right to move, and thereby invalidate, the position in memory. – Silvio Mayolo Jun 12 '18 at 22:28
  • @SilvioMayolo Right! So shoudl I do a `malloc()`,`memcpy()`,`free()` -combo instead? – stdcerr Jun 12 '18 at 22:31
  • 1
    That's no better. Your `map` is storing pointers to particular places in memory, and `realloc` or `malloc` and `memcpy` are moving the data you pointed at to different places in memory. You would have to go back through the `map` and update all of the pointers with the new locations. `map`, and all of the standard library containers, are at their best when they contain data directly. If this cannot be done, you have to make sure what you are pointing at does not move. – user4581301 Jun 12 '18 at 22:43
  • @user4581301 Ah alright, I can make them containing thedata directly (instead of using pointers, yes). I just thought using pointers would be more efficient & faster... (I'm coming from `C`, though). So I assume I can leave the global, static and pointer magic (and memory allocations) away and instead store all my `struct` data directly in the `std::map` - If this is what works, please convert the comment into an answer and I'll accept! Thanks! – stdcerr Jun 12 '18 at 22:46
  • 1
    Sometimes it is faster, but when you have two data structures referring to the same location, you have to be really careful that one does not invalidate the other. This would be the same in C, but in C++ destructors can make things both better (encapsulates cleanup) and more difficult (if you destroy one copy of an object and it cleans up, what of the other copies?) at the same time. Recommended reading: [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) – user4581301 Jun 12 '18 at 22:50

1 Answers1

0

For a working solution, I replaced the global static variable and the pointers in the map with data that is located directly in the map. Plus I removed the typedef from the struct and declared it the C++ way instead, as in:

std::map<std::string, MyType> *pmap

struct MyType {
     long long timestamp;
     int cnt;
     bool parked;
 };
stdcerr
  • 13,725
  • 25
  • 71
  • 128
  • New question: How sure are you that you need to dynamically allocate the `map`? Unless you have an unknown lifespan, you're better off with `std::map pmap;` and passing it around by reference: `int map_add_item(std::map &pmap, std::string str, long long tmestmp)` A reference is not a pointer, a restricted pointer, or anything like that. It might not exist as anything but a name for a variable that's somewhere else in the code. The compiler can perform some surprisingly effective optimizations with references that it can't with pointers. – user4581301 Jun 13 '18 at 01:10
  • @user4581301 I don't dynamicall allocate the map, it's declared as a private global member of the class but I'll pass it between functions in the prototype so that it's easier to follow the ata flow for anyone who comes after me... (avoid use of globals in data modifier functions) – stdcerr Jun 13 '18 at 03:44