Solved

stl map insert problem

Posted on 2004-04-15
22
2,427 Views
Last Modified: 2013-12-14
Environment:  Windows 2000, VS6 SP5, default MS stl implementation

I have  a trivial class, implemented in a DLL, as follows:
#ifndef TRIVIAL_H
#define TRIVIAL_H

class AFX_EXT_CLASS aff_data
{
public:
      aff_data();
      aff_data( const aff_data & rhs );
      const aff_data & operator =( const aff_data & rhs );

      int cb_index;
      std::string aff_name;
};

AFX_EXT_CLASS void dump_affiliates( const aff_map_t & aff );   // <- from the header file

typedef std::map<long, aff_data> aff_map_t;

#endif //TRIVIAL_H

In the same DLL is a diagnostic dump function for aff_map_t:

void dump_affiliates( const aff_map_t & aff )
{
      for( aff_map_t::const_iterator iter2 = aff.begin();
                  iter2 != aff.end();
                  iter2++ )
      {
            TRACE1( "aff_num = %ld  ", (*iter2).first );
            TRACE1( "name = '%s'  ", (*iter2).second.aff_name.substr(0,20).c_str() );
            TRACE1( "cb_index = %d\n", (*iter2).second.cb_index );
      }
}



In a separate module that links against the aff_data class DLL, I populate the map as follows:

aff_map_t affdata;

for( ; ; )
{
      CString aff_num;
      CString aff_name;
      if( ! create_aff( aff_num, aff_name ) )
            break;

      long x = atol( aff_num );
      aff_data a;
      a.cb_index = -1;
      a.aff_name = (LPCTSTR) aff_name;
      affdata[ x ] = a;
}


Immediately after the loop terminates, I dump the map container to debug output, and after the first map item is read, I get an access violation, apparently as a result of iter2 (see above) pointing to an invalid or corrupt map tree item.

dump_affiliates( affdata );

Any ideas ?  It seems like this should work.

0
Comment
Question by:xn12z9i
  • 9
  • 5
  • 4
  • +2
22 Comments
 
LVL 12

Expert Comment

by:stefan73
ID: 10840499
Hi xn12z9i,


> TRACE1( "name = '%s'  ", (*iter2).second.aff_name.substr(0,20).c_str() );

Iterators and c_str() don't match nicely.

Try replacing this with
string tmp=(*iter2).second.aff_name;
TRACE1( "name = '%s'  ", tmp.substr(0,20).c_str() );
(or use
TRACE1( "name = '%.20s'  ", tmp.c_str() );
)



Cheers,
Stefan
0
 
LVL 6

Expert Comment

by:Mafalda
ID: 10841371
1) what is create_aff doing ?

2) Did you try to trace the values when inserted to aff_map_t ?

3) I assume that the proble is in

a.aff_name = (LPCTSTR) aff_name;

change it to copy th contents of aff_name ... you copy here the pointer !


4) naming is really confusing ;o)
0
 

Author Comment

by:xn12z9i
ID: 10842297
Stefan73:

Thanks for the tip about iterators/.c_str(), but no go. The problem appears to be that the iterator itself is pointing to a completely invalid object after the first increment of the iterator. It almost appears as though the map [] operator inserts the new key/value pair, but my overloaded assignment operator is failing to copy the values from "a" to the pair<>.second reference returned by operator []. What I'll try to do next is save the reference to the returned pair<>.second and assign into that (possibly saving the address of the pair<>.second object as well for further debugging).

Mafalda:

1. create_aff() is a trivial function that populates the CString arguments.
2. Well, yes and no, it's slightly confusing stepping through the truncated variable names and inlines in the stl headers. Yes, immediately after the insert, a TRACE on :

TRACE1( "%s\n", (affdata[(newly inserted key)]).aff_name.c_str() );

works perfectly fine, but only immediately after the insert.  affdata.size() returns a count that indicates the item was inserted. I'll try to debug a little further.

3. a.aff_name = (LPCTSTR) aff_name is completely valid.  The std::string assignment operator supports assignment of "const char *" types.

Thanks for the replys. Anyone else have any ideas ?
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 10843179
You use

>> aff_map_t::const_iterator iter2

try to use

   aff_map_t::iterator iter2

instead.

Regards, Alex
0
 
LVL 17

Expert Comment

by:rstaveley
ID: 10843379
This looks dubious to me:

--------8<--------
for( ; ; )
{
     // ..
     CString aff_name;
     // ...
     a.aff_name = (LPCTSTR) aff_name;
     // ...
}
--------8<--------

You are getting a const char* to a temporary object from local object aff_name. When aff_name goes out of scope, I'd expect the LPCTSTR to be a wild pointer.
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 10843442
>> You are getting a const char* to a temporary object from local object aff_name.

As a.aff_name is      

     std::string aff_name;  

std::string::operator =  () makes a copy of the CString (internal) char array. So, there is nothing to worry about.

Regards, Alex
0
 
LVL 39

Expert Comment

by:itsmeandnobodyelse
ID: 10843504
As CString has  a - so-called - cast operator LPCTSTR() and there is a string constructor and assign operator that takes a right-hand LPCTSTR (or const char*) a really *good* compiler wouldn't need an explicit cast.

Regards, Alex
0
 
LVL 17

Expert Comment

by:rstaveley
ID: 10843690
Thanks, Alex, I hadn't looked at the class definition properly.
0
 
LVL 6

Expert Comment

by:Mafalda
ID: 10843694
The LPCTSTR seems valid alright ...  although it is a temporary pointer.
It wouldn't hurt to try and copy its contents . . . I say so from experience . . .

"In the simplest case, you can cast a CString object to be an LPCTSTR. The LPCTSTR type conversion operator returns a pointer to a read-only C-style null-terminated string from a CString object.
The pointer returned by LPCTSTR points into the data area used by the CString. If the CString goes out of scope and is automatically deleted or something else changes the contents of the CString, the LPCTSTR pointer will no longer be valid. Treat the string to which the pointer points as temporary. "

0
 
LVL 17

Expert Comment

by:rstaveley
ID: 10843743
I notice that aff_data has a copy constructor and assignment operator defined, which is probably not necessary bearing in mind the members. Could we see these in case there is something untoward happening in them?
0
 
LVL 17

Expert Comment

by:rstaveley
ID: 10843760
> Treat the string to which the pointer points as temporary

Yes, but the temporary will persist for the duration of the assignment.
0
What Is Threat Intelligence?

Threat intelligence is often discussed, but rarely understood. Starting with a precise definition, along with clear business goals, is essential.

 

Author Comment

by:xn12z9i
ID: 10843949
rstavely, copy/assignment implementation follows:

aff_data::aff_data() : aff_name( "" ), cb_index(-1)
{
}

aff_data::aff_data( const aff_data & rhs ) : aff_name(rhs.aff_name), cb_index(rhs.cb_index)
{
}

const aff_data & aff_data::operator =( const aff_data & rhs )
{
      if( this == &rhs )
            return *this;

      cb_index = rhs.cb_index;
      aff_name = rhs.aff_name;
      return *this;
}
0
 

Author Comment

by:xn12z9i
ID: 10844041
Looking at the debug output a little more closely, here is what I get at the point of the fault:

aff_num = 744  name = 'test string 1' cb_index = -1
aff_num = -842150451  

The fault message is that the memory at address '0x10489f21' refrerenced memory at '0xcdcdcdcc'.

As 0xcdcdcdcc looks pretty regularly formed, am I tripping over a section of previously initialized memory, or some kind of memory block boundary mark ?

Alex, re iterator vs. const_iterator, that did not have any noticeable effect. I still get the exception. Thanks, though.
0
 
LVL 6

Expert Comment

by:Mafalda
ID: 10844156
So it is as I suspected either create_aff is doing something wrong or the LPCTSTR convertion is bad ...
0
 

Author Comment

by:xn12z9i
ID: 10844333
As a proof-of-concept, I converted the container to std::list<aff_data> instead of std::map<long,aff_data>. dump_affiliates() works properly now. Obviously, std::map<> is not playing nice with my aff_data class implementation. Does anyone have any insight into the std::map::insert() semantics ?

I also tried converting the map to std::map<long, aff_data*>, but still got the same access violation errors. Is the red/black tree in the std::map<> implementation getting corrupted somehow ?

Mafalda,

create_aff() is trivial for the code snippet I posted. It simply populates two CString objects passed as arguments.  As to your second point, the LPCTSTR conversion is no different than

std::string buf;
buf = "teststring";

in terms of rvalue object lifetime, scope, and assignment semantics.
0
 
LVL 17

Expert Comment

by:rstaveley
ID: 10844351
0xcdcdcdcd is uninitialised memory (see http://www.microsoft.com/msj/1198/c/c1198.aspx). It looks like you've decremented an integer in uninitialised memory, if you see 0xcdcdcdcc.
0
 
LVL 17

Expert Comment

by:rstaveley
ID: 10844352
> 0xcdcdcdcd is uninitialised memory

In debug mode on MSVC.
0
 
LVL 17

Expert Comment

by:rstaveley
ID: 10844566
> Does anyone have any insight into the std::map::insert() semantics ?

affdata[ x ] = a;

...should be OK. It takes a copy of a.
0
 
LVL 17

Expert Comment

by:rstaveley
ID: 10844620
What do you see if you display the aff_nums used for the index in the map entries?

for( ; ; )
{
     // ...
#ifdef _DEBUG
    afxDump << "Key " << x << '\n';
#endif
     affdata[ x ] = a;
     // ...
}
0
 
LVL 6

Expert Comment

by:Mafalda
ID: 10844624
It is not that I do understand the code or your explanations I am just saying that something is malfunctioning  . . . I am sure it is in the allocation of memory for the aff_name variable or somewhere in the way. You also show this is the problem.
In my honest opinion it is usualy something in the programmers code ... even if he is sure his/her code is OK.
In other cases the bug is caused by a bug in the compiler ...
Maybe you should try to compile it in VS .NET ...
Everybody seems to say the code is OK so it seems like a VC++ 6 bug.

"0xcdcdcdcd is uninitialised memory" -> It's obvious that either you or Microsoft code is not initializing memory in the right way !!!!
You do not have to be Einstein to understand that ;o)
0
 
LVL 17

Accepted Solution

by:
rstaveley earned 250 total points
ID: 10844901
One last thought (before I shoot off for the weekend)...

Your assignment operator implementation code is in the DLL, but you are assigning aff_name in your calling code.

That means that you are going to get free store memory for aff_name's data in the calling code (somewhere or other STL is going to call new), but you are releasing it in the DLL (somewhere or other STL is going to call delete).

The DLL and calling code have separate heap managers. I would expect that the heap manager in the DLL would choke at deleting a pointer allocated by the heap manager in the calling code.

To test this theory, can you try implementing the assignment operator code inline and see if that fixes it?

i.e. in your header:
--------8<--------
inline const aff_data & aff_data::operator =( const aff_data & rhs )
{
     if( this == &rhs )
          return *this;

     cb_index = rhs.cb_index;
     aff_name = rhs.aff_name;
     return *this;
}
--------8<--------

If that does the job, you should consider replacing the assignment...

>   a.aff_name = (LPCTSTR) aff_name;

with a setter which is implemented by the DLL, and make the data members private and of course remove all of the inlining.

Must go.... the weekend beckons.... hope this helps.
0
 

Author Comment

by:xn12z9i
ID: 10844986
Just an update - my implementation works, no modifications required, when using STLPort...

Thanks for all the help so far.
0

Featured Post

How to improve team productivity

Quip adds documents, spreadsheets, and tasklists to your Slack experience
- Elevate ideas to Quip docs
- Share Quip docs in Slack
- Get notified of changes to your docs
- Available on iOS/Android/Desktop/Web
- Online/Offline

Join & Write a Comment

Introduction This article is a continuation of the C/C++ Visual Studio Express debugger series. Part 1 provided a quick start guide in using the debugger. Part 2 focused on additional topics in breakpoints. As your assignments become a little more …
Basic understanding on "OO- Object Orientation" is needed for designing a logical solution to solve a problem. Basic OOAD is a prerequisite for a coder to ensure that they follow the basic design of OO. This would help developers to understand the b…
The viewer will learn how to pass data into a function in C++. This is one step further in using functions. Instead of only printing text onto the console, the function will be able to perform calculations with argumentents given by the user.
The viewer will be introduced to the member functions push_back and pop_back of the vector class. The video will teach the difference between the two as well as how to use each one along with its functionality.

705 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

20 Experts available now in Live!

Get 1:1 Help Now