Problem in deleting entries from Hash_set

[Prafulla T]

Is following correct way of deleting entries from stl hashset.
Will it create any memory corruption ?
One of my program uses this way and it is crashing in strange way but
purify or valgrind does not
show up anything. I am suspecting this piece of code as a problem.
Can anyone comment on it ?

for (hash_set<UserType*>::iterator hashIt = hashSet.begin();
hashIt != hashSet.end(); hashIt++)
{
UserType *pUserType = *hashIt;
hashIt++;
delete pUserType;
pUserType = NULL;
}

 

[srdgame]

于 Fri, 06 Mar 2009 13:02:17 -0800，Prafulla T写到：

Is following correct way of deleting entries from stl hashset. Will it
create any memory corruption ? One of my program uses this way and it is
crashing in strange way but purify or valgrind does not
show up anything. I am suspecting this piece of code as a problem. Can
anyone comment on it ?

for (hash_set<UserType*>::iterator hashIt = hashSet.begin();
hashIt != hashSet.end(); hashIt++)
{
UserType *pUserType = *hashIt;
hashIt++;
delete pUserType;
pUserType = NULL;
}

Anyway, according to my understanding that you are deleting the UserType
objects but not any item of your hashSet.

And the crash could came from the DOUBLE "hashIt++". As I can image your
code should either in this:
for (hash_set<UserType*>::iterator ptr = hashSet.being(); ptr !=
hashSet.end(); )
{
UserType* pUserType = *ptr;
ptr++;
delete pUserType;
// pUserType = NULL // you no longer need to set it to NULL,
since this is local pointer.
}
// You have to clear the hashSet
hashSet.clear();

Or in this:
for (hash_set<UserType*>::iterator ptr = hashSet.being(); ptr !=
hashSet.end(); ++ptr)
{
UserType* pUserType = *ptr;
// ptr++; // Since you are deleteing the UserType object which
is referenced by ptr, but ptr is still valid after you deleted the
UserType objects.
delete pUserType;
// pUserType = NULL // you no longer need to set it to NULL,
since this is local pointer.
}
// You have to clear the hashSet
hashSet.clear();

Yours,
srdgame

 

[ZikO]

for (hash_set<UserType*>::iterator hashIt = hashSet.begin();
hashIt != hashSet.end(); hashIt++)
{
UserType *pUserType = *hashIt;
hashIt++;
delete pUserType;
pUserType = NULL;
}

OK. First of all you cannot use hash_set.begin(). hash_set<> is the type
not instantiated object! If this worked, that code below would also work
!!!
template <class T>
struct A {
void begin() {}
};
int main() {
//! A.f(); // very baaaad
A<int> a;
a.f(); // correct
}

So you have to have an instantiated object of hash_set<UserType*>
somewhere ... let's say it's:

hash_set<UserType*> hsutp;

Before I write the code a few things to be said.
* you increments iterator two times, in for definition and in code
inside the loop !!!
* you are trying to delete object pointed by pUserType, which has not
been created dynamically by operator new !!!
* you compare pointers of two different objects, UserType with container
hash_set !!!

Now having said that all, if I understand you, your code should look
like below (roughly):

// ...
hash_set<UserType*> hsutp; // it can be any name used by you
// I assume u have done something on
// this object !!!
// ...
for (hash_set<UserType*>::iterator hashIt = hsutp.begin();
hashIt != hsutp.end(); ++hashIt) {
delete *hashIt;
*hashIt = 0;
}

Best.

 

[ZikO]

> for (hash_set<UserType*>::iterator hashIt = hashSet.begin();
> hashIt != hashSet.end(); hashIt++)
> {
> UserType *pUserType = *hashIt;
> hashIt++;
> delete pUserType;
> pUserType = NULL;
> }

OK. First of all you cannot use hash_set.begin(). hash_set<> is the type
not instantiated object! If this worked, that code below would also work !!!
template <class T>
struct A {
void begin() {}
};
int main() {
//! A.f(); // very baaaad
A<int> a;
a.f(); // correct
}

So you have to have an instantiated object of hash_set<UserType*>
somewhere ... let's say it's:

hash_set<UserType*> hsutp;

Before I write the code a few things to be said.
* you increments iterator two times, in for definition and in code
inside the loop !!!
* you are trying to delete object pointed by pUserType, which has not
been created dynamically by operator new !!!
* you compare pointers of two different objects, UserType with container
hash_set !!!

Now having said that all, if I understand you, your code should look
like below (roughly):

// ...
hash_set<UserType*> hsutp; // it can be any name used by you
// I assume u have done something on
// this object !!!
// ...
for (hash_set<UserType*>::iterator hashIt = hsutp.begin();
hashIt != hsutp.end(); ++hashIt) {
delete *hashIt;
*hashIt = 0;
}

Best.

 

[ZikO]

for (hash_set<UserType*>::iterator hashIt = hashSet.begin();
hashIt != hashSet.end(); hashIt++)
{
UserType *pUserType = *hashIt;

I would get rid of this line completely.

hashIt++;

Are u aware you increment iterator twice!

delete pUserType;

I would just use:

delete *hashIt
*hashIt = 0; // zero to pointers no pointing anything

pUserType = NULL;

you assigning zero to pointer, which is going to be deleted, in every
step of loop. I would simply use iterator to indicate what delete should
free. So I would do something like below:

// code
typedef hash_set<UserType*>::iterator It;
for(It hashIt = hashSet.begin(); hashIt != hashSet.end(); ++hashIt) {
delete *hashIt;
*hashIt = 0;
}
// end of code

 

[Prafulla T]

Prafulla T wrote:

 >     for (hash_set<UserType*>::iterator hashIt = hashSet.begin();
 >          hashIt != hashSet.end(); hashIt++)
 >     {
 >         UserType *pUserType = *hashIt;
 >         hashIt++;
 >         delete pUserType;
 >         pUserType = NULL;
 >     }

OK. First of all you cannot use hash_set.begin(). hash_set<> is the type
not instantiated object! If this worked, that code below would also work !!!
template <class T>
struct A {
    void begin() {}};

int main() {
    //! A.f();   // very baaaad
    A<int> a;
    a.f(); // correct

}

hashSet was the name of hash_set object.
It is not clear to me how it would cause the problem.
And yes, incrementing iterator twice was a problem. I think I put this
while writing the mail. it was not actually there in the code. (It was
not in "for loop" in actual code).

I debugged it further and found that cause was somewhere else.
Of course, i missed to clear the hash_set after deleting the elements.
It didn't cause any trouble in my case.

* you are trying to delete object pointed by pUserType, which has not
been created dynamically by operator new !!!

It was created dynamically by operator new. I just didn't add that
code here.

 

[James Kanze]

Is following correct way of deleting entries from stl hashset.
Will it create any memory corruption ?
One of my program uses this way and it is crashing in strange way but
purify or valgrind does not
show up anything. I am suspecting this piece of code as a problem.
Can anyone comment on it ?

for (hash_set<UserType*>::iterator hashIt = hashSet.begin();
hashIt != hashSet.end(); hashIt++)
{
UserType *pUserType = *hashIt;
hashIt++;
delete pUserType;
pUserType = NULL;
}

I don't know; it's hard to say without more context. But there
are at least two problems: you're incrementing the iterator
twice, so if the table contains an odd number of entries, you'll
run off the end (into undefined behavior); and you're deleting
what the pointer in the table points to while leaving the
pointer in the table (you only null a local variable which
immediately goes out of scope)---if you do just about anything
with the pointer later, it's undefined behavior.

 

[ZikO]

You have answered on my previous post which was wrong i tried to cancel
that as quickly as possible (hadn't enough time though ), I misread
your code (details about hash_set container and its object hashSet). I
gave the proper answer afterwards.

All in all, you should give us full code because without it it's really
difficult to guess what's causing problems. My bad was to try help you
based on part of the code.

Now I know I should not do thatagain. Many apologies =)


best.

 

[Joe Greer]

Is following correct way of deleting entries from stl hashset.
Will it create any memory corruption ?
One of my program uses this way and it is crashing in strange way but
purify or valgrind does not
show up anything. I am suspecting this piece of code as a problem.
Can anyone comment on it ?

for (hash_set<UserType*>::iterator hashIt = hashSet.begin();
hashIt != hashSet.end(); hashIt++)
{
UserType *pUserType = *hashIt;
hashIt++;
delete pUserType;
pUserType = NULL;
}

Hmmm, does your hash_set::erase() perhaps return an iterator? If so, the
loop you want probably looks something like.

for (hash_set<UserType *>::iterator hashIt = hashSet.begin();
hashIt != hashSet.end()
{
UserType *pUserType = *hashIt;
hashIt = hashSet.erase(hashIt);
delete pUserType;
}


That should delete every item in your hash_set and delete the elements as
well.

HTH

joe