Regarding STL-Map

[Dew]

Hi all,
I needed a clue to get rid of this problem. Code work fine for few run
but then fail with some segmentation fault error with next subsequent
run. I am using Visual Age C++ under AIX compiler , version 5

1 //******************************************************************8
2 //1. some other stuff goes here
3 //2. some other stuff goes here
4 //3. some other stuff goes here
5 // Now here is the Map initializaiton steps ..............
6 map<IString,IString> myMap ;
7 if (myMap.find((IString)name) != myMap.end()) // if Header Exist in
Map Table
8 {
9 IString s1;
10 for ( map< IString, IString >::const_iterator im = myMap.begin();
im != myMap.end(); ++im )
11 if (im->first==name) s1=(IString)im->second ;
12 }
13 else
14 myMap[(IString)name]=(IString)value;
//******************************************************************


Here value and name is type of const char*

for the first few run Map is being initialize properly but suddenly
some memory corruption error shown & it's stuck at line 4 only. any
suggestion or clue on this please ?

 

[Andre Kostur]

Hi all,
I needed a clue to get rid of this problem. Code work fine for few run
but then fail with some segmentation fault error with next subsequent
run. I am using Visual Age C++ under AIX compiler , version 5

1 //******************************************************************8
2 //1. some other stuff goes here
3 //2. some other stuff goes here
4 //3. some other stuff goes here
5 // Now here is the Map initializaiton steps ..............
6 map<IString,IString> myMap ;
7 if (myMap.find((IString)name) != myMap.end()) // if Header Exist in
Map Table
8 {
9 IString s1;
10 for ( map< IString, IString >::const_iterator im = myMap.begin();
im != myMap.end(); ++im )
11 if (im->first==name) s1=(IString)im->second ;

What's the point of this additional loop? You've already found name
inside of myMap, why not store the result of your find() call and change
the value through that iterator?

Also, you should prefer to use C++-style casts to C-style casts.

12 }
13 else
14 myMap[(IString)name]=(IString)value;
//******************************************************************


Here value and name is type of const char*

Is there a constructor from const char * to IString? That would
eliminate a lot of your casts (some of which are already superfluous).

for the first few run Map is being initialize properly but suddenly
some memory corruption error shown & it's stuck at line 4 only. any
suggestion or clue on this please ?

And if it's stuck at line 4, how can we diagnose since line 4 is a
comment where "some other stuff" happens?

 

[Dew]

Hi Andre,
Sorry that above code snap is not very clear..
What I want to do is :

if the Header is found, I need to concatenate the values for this
header . But if the header is new for the Map, I need to simply assign
this Header and it's corresponding values. This is why I have loop
inside the if statement.
so the final map will be look like this...
key1:value1.1,value1.2,value1.3;
key2:value2.1,value2.2,value2.3;

Here's the more clear lines....

1 //******************************************************************8
2
3
4
5
6 map<IString,IString> myMap ;
7 if (myMap.find((IString)name) != myMap.end()) // if Header Exist in
Map Table
8 {
9 IString s1;
10 for ( map< IString, IString >::const_iterator im = myMap.begin();
im != myMap.end(); ++im )
11 if (im->first==name) s1=(IString)im->second ;

if(strstr(s1, (IString)value)==NULL)
IString s2=s1 + (IString)"," + (IString)value;
myMap[(IString)evv_name]=s2;
12 }
13 else
14 myMap[(IString)name]=(IString)value;
//******************************************************************



during the step 2 to 5 I am getting the value of "value" and "name" in
the const char * string. and that's why I have to convert this into
IString here. Now my prob is Map is being initialized good for few run
but not always and hanging just before the step 6.

Just wanted to mention that all the above stuff are inside the for loop
who is being executed for each Key (here it's "name") & Value ( here
it's "value")

Is there some problem in casting the char * with IString or I am
missing something else related to Map .. ?

 

[Andre Kostur]

Hi Andre,
Sorry that above code snap is not very clear..
What I want to do is :

if the Header is found, I need to concatenate the values for this
header . But if the header is new for the Map, I need to simply assign
this Header and it's corresponding values. This is why I have loop
inside the if statement.
so the final map will be look like this...
key1:value1.1,value1.2,value1.3;
key2:value2.1,value2.2,value2.3;

Here's the more clear lines....

1 //******************************************************************8
2
3
4
5
6 map<IString,IString> myMap ;
7 if (myMap.find((IString)name) != myMap.end()) // if Header Exist in
Map Table
8 {
9 IString s1;
10 for ( map< IString, IString >::const_iterator im = myMap.begin();
im != myMap.end(); ++im )
11 if (im->first==name) s1=(IString)im->second ;

if(strstr(s1, (IString)value)==NULL)
IString s2=s1 + (IString)"," + (IString)value;
myMap[(IString)evv_name]=s2;
12 }
13 else
14 myMap[(IString)name]=(IString)value;
//******************************************************************

Right, but inside your if statement, you're going and iterating through
the map again to find the location that you've already found by calling
find(). Why not store the iterator returned by find() and use that
instead of searching the map again?

during the step 2 to 5 I am getting the value of "value" and "name" in
the const char * string. and that's why I have to convert this into
IString here. Now my prob is Map is being initialized good for few run
but not always and hanging just before the step 6.

But you still haven't shown us what's before step 6! We can only guess
as to what's going on up there.

Just wanted to mention that all the above stuff are inside the for loop
who is being executed for each Key (here it's "name") & Value ( here
it's "value")

Is there some problem in casting the char * with IString or I am
missing something else related to Map .. ?

No clue what an IString actually is... so can't say much here...

 

[Tom Widmer]

Hi Andre,
Sorry that above code snap is not very clear..
What I want to do is :

if the Header is found, I need to concatenate the values for this
header . But if the header is new for the Map, I need to simply assign
this Header and it's corresponding values. This is why I have loop
inside the if statement.
so the final map will be look like this...
key1:value1.1,value1.2,value1.3;
key2:value2.1,value2.2,value2.3;

Here's the more clear lines....

1 //******************************************************************8
2
3
4
5
6 map<IString,IString> myMap ;
7 if (myMap.find((IString)name) != myMap.end()) // if Header Exist in
Map Table
8 {
9 IString s1;
10 for ( map< IString, IString >::const_iterator im = myMap.begin();
im != myMap.end(); ++im )
11 if (im->first==name) s1=(IString)im->second ;

if(strstr(s1, (IString)value)==NULL)
IString s2=s1 + (IString)"," + (IString)value;
myMap[(IString)evv_name]=s2;
12 }
13 else
14 myMap[(IString)name]=(IString)value;

I'll rewrite this using better style:

typedef map<IString,IString> map_t;
typedef map_t::iterator iterator_t;
map_t myMap;
iterator_t pos = myMap.find(name);
if (pos != myMap.end() && strstr(pos->second, value) == NULL)
{
pos->second += IString(",") + value;
}
else
{
myMap.insert(map_t::value_type(name, value));
}

Why are you using "IString" rather than std::string? Better than the
above is:
typedef map<string,string> map_t;
typedef map_t::iterator iterator_t;
map_t myMap;
iterator_t pos = myMap.find(name);
if (pos != myMap.end() && pos->second.find(value) == string::npos)
{
pos->second += string(",") + value;
}
else
{
myMap.insert(map_t::value_type(name, value));
}

As you can see, there is no need for a loop in either case.

Tom

 

[Daniel T.]

Hi Andre,
Sorry that above code snap is not very clear..
What I want to do is :

if the Header is found, I need to concatenate the values for this
header . But if the header is new for the Map, I need to simply assign
this Header and it's corresponding values. This is why I have loop
inside the if statement.
so the final map will be look like this...
key1:value1.1,value1.2,value1.3;
key2:value2.1,value2.2,value2.3;

Here's the more clear lines....

1 //******************************************************************8
2
3
4
5
6 map<IString,IString> myMap ;

If you just created myMap in line 6, there is no need for the find
because you know nothing is in the map.

7 if (myMap.find((IString)name) != myMap.end()) // if Header Exist in
Map Table
8 {
9 IString s1;
10 for ( map< IString, IString >::const_iterator im =
myMap.begin();
im != myMap.end(); ++im )
11 if (im->first==name) s1=(IString)im->second ;

if(strstr(s1, (IString)value)==NULL)
IString s2=s1 + (IString)"," + (IString)value;
myMap[(IString)evv_name]=s2;
12 }
13 else
14 myMap[(IString)name]=(IString)value;
//******************************************************************

It would help immeasurably if you could paste code that we can simply
copy and paste in a cpp file, run it and see the problem.

during the step 2 to 5 I am getting the value of "value" and "name" in
the const char * string. and that's why I have to convert this into
IString here. Now my prob is Map is being initialized good for few run
but not always and hanging just before the step 6.

I doubt the code you are posting is where the error is. I suspect it's
more like a memory overwrite somewhere else in your code.

Just wanted to mention that all the above stuff are inside the for loop
who is being executed for each Key (here it's "name") & Value ( here
it's "value")

Is line 6 in the for loop? If so, then there's a problem...

Is there some problem in casting the char * with IString or I am
missing something else related to Map .. ?

Who wrote IString? Are you null terminating your char*?

 

[Daniel T.]

I'll rewrite this using better style:

typedef map<IString,IString> map_t;
typedef map_t::iterator iterator_t;
map_t myMap;
iterator_t pos = myMap.find(name);
if (pos != myMap.end() && strstr(pos->second, value) == NULL)
{
pos->second += IString(",") + value;
}
else
{
myMap.insert(map_t::value_type(name, value));
}

In your code, if the value isn't already in the map, you still have to
make a second search. In the code below, only one search through the map
is necessary...

typedef string IString; // for those of us who don't have IString

void add_value( map<IString,IString>& myMap, IString key, IString value )
{
map<IString, IString>::iterator it = myMap.lower_bound( key );
if ( it->first == key )
it->second += IString(",") + value;
else
myMap.insert( it, make_pair( key, value ) );
}

 

[Tom Widmer]

In your code, if the value isn't already in the map, you still have to
make a second search.

True, it is suboptimal.

In the code below, only one search through the map

is necessary...

typedef string IString; // for those of us who don't have IString

void add_value( map<IString,IString>& myMap, IString key, IString value )
{
map<IString, IString>::iterator it = myMap.lower_bound( key );
if ( it->first == key )
it->second += IString(",") + value;
else
myMap.insert( it, make_pair( key, value ) );
}

A slightly more compact alternative would be:

IString& ref = myMap[key];
if (ref.length() > 0)
ref += ',';
ref += value;

However, the OP appeared to want to check that the value wasn't already
present in the value string. So:

IString& ref = myMap[key];
if (!ref.includes(value)) //assuming this is IBM's IString
{
if (ref.length() > 0)
ref += ',';
ref += value;
}

Tom

 

[Dew]

Thanks all, I've updated the Map operation as you all suggested &
surely it's more efficient way.

Finally I could able to locate the prob. The prob was with IString
object. IString is Visual age C++ Class that provide String operation.

Just wanted to share the prob.
to Initialize the IString, i was doing something this way...
IString Name = NULL ;
or
IString Value= IString("") ;

It was okie for first few run but later on subsequent run it was
creating a problem , probably Stack/Heap memory assigned by IString
object was being corrupted bcoz improper initialization.

To solve the above, I did like...
IString Name= IString("",512);

[ "When you create an object of a Istring class, the actual characters
that make up the string are not stored in the string object. Instead,
the characters are stored in an object of a buffer class. Each IString
holds a pointer to an object of the OCL class IBuffer."]


Thank you all for good suggestion.