Cleanup Technique

[brad.power]

Hi All,

Often in C++ code I find myself having to allocate character arrays to
pass back to callers for error codes, or as topic/data in message
passing/observer type scenarios (When having them on the heap is
necessary to avoid them going out of scope before something else, like
another thread, has had a chance to process them). What I wanted was a
neat way to keep track of these allocations within the class, and
clean them all up nicely, without too much overhead.

What I hit upon is based on the following ideas:
1. std::string is just 'better' than char*, so use it!
2. std::vector, in its destructor, will attempt to destroy each object
within it.

So what I did was to declare a vector of std::strings inside
'SomeClass' - a class which needs to keep track of a lot of strings it
creates during its adventures:

class SomeClass
{
....
private:
std::vector<std::string> m_stringvector;
....
}

And then, in the body of some method in SomeClass:

std::string* s1 = new std::string();
std::string* s2 = new std::string();
//do something with s1 and s2 - i.e. set them and pass them back to a
function
//now add them to the class vector to keep track of them:
m_stringvector.push_back(*s1);
m_stringvector.push_back(*s2);

So the idea is that m_stringvector is just a container to keep track
of all of my allocated strings, and when SomeClass destructs,
m_stringvector will automatically free all the string objects.

To me, this seems like a good way to keep track, as long as:
1. strings created this way are not freed anywhere else
2. The program design is happy to have all the strings destruct when
the class destructs
3. The life of SomeClass is such that it will never end up holding
zillions of strings, hogging memory
4. The programmer remembers to push the strings onto the vector

Since I stumbled across this in my own programming, and havent seen it
in a book or as a documented technique, I wanted to run it past the
group for any comments you may have. Is it as good an idea as I think?
Are there any obvious (or not so obvious) gotchas or 'you shouldn't do
that' aspects to this? Is this an actual documented technique that I
am just ignorant of?

Thanks
Brad

 

[Phlip]

Often in C++ code I find myself having to allocate character arrays to
pass back to callers for error codes, or as topic/data in message
passing/observer type scenarios (When having them on the heap is
necessary to avoid them going out of scope before something else, like
another thread, has had a chance to process them). What I wanted was a
neat way to keep track of these allocations within the class, and
clean them all up nicely, without too much overhead.

C++ throws objects by copy, so all you need are valid copy semantics.

What I hit upon is based on the following ideas:
1. std::string is just 'better' than char*, so use it!

Always follow that rule, for nearly all strings everywhere.

2. std::vector, in its destructor, will attempt to destroy each object
within it.

Yes, but destroy does not mean delete.

std::string* s1 = new std::string();
std::string* s2 = new std::string();

Why the new?

//do something with s1 and s2 - i.e. set them and pass them back to a
function
//now add them to the class vector to keep track of them:
m_stringvector.push_back(*s1);
m_stringvector.push_back(*s2);

Make m_stringvector store strings, and just push them in!

1. strings created this way are not freed anywhere else

Treat the string object as an object; it will take care of freeing its
controlled array.

However, your m_stringvector will not natively delete all its strings, so
they will leak. Again: Just store, copy, and throw strings, not pointers to
string objects, and let them take care of their own low-level details.

 

[Alf P. Steinbach]

* (e-mail address removed):

Hi All,

Often in C++ code I find myself having to allocate character arrays to
pass back to callers for error codes, or as topic/data in message
passing/observer type scenarios (When having them on the heap is
necessary to avoid them going out of scope before something else, like
another thread, has had a chance to process them). What I wanted was a
neat way to keep track of these allocations within the class, and
clean them all up nicely, without too much overhead.

What I hit upon is based on the following ideas:
1. std::string is just 'better' than char*, so use it!
2. std::vector, in its destructor, will attempt to destroy each object
within it.

So what I did was to declare a vector of std::strings inside
'SomeClass' - a class which needs to keep track of a lot of strings it
creates during its adventures:

class SomeClass
{
...
private:
std::vector<std::string> m_stringvector;
...
}

OK so far, except missing semicolon.

And then, in the body of some method in SomeClass:

std::string* s1 = new std::string();
std::string* s2 = new std::string();
//do something with s1 and s2 - i.e. set them and pass them back to a
function
//now add them to the class vector to keep track of them:
m_stringvector.push_back(*s1);
m_stringvector.push_back(*s2);

Here you have needless dynamic memory allocation and a memory leak.

s1 and s2 will not be deallocated.

Why don't you declare s1 and s2 directly as sd::string.


Cheers, & hth.,

- Alf

 

[brad.power]

C++ throws objects by copy, so all you need are valid copy semantics.


Always follow that rule, for nearly all strings everywhere.


Yes, but destroy does not mean delete.


Why the new?


Make m_stringvector store strings, and just push them in!


Treat the string object as an object; it will take care of freeing its
controlled array.

However, your m_stringvector will not natively delete all its strings, so
they will leak. Again: Just store, copy, and throw strings, not pointers to
string objects, and let them take care of their own low-level details.

--
Phlip
http://www.oreilly.com/catalog/9780596510657/
^ assert_xpath
http://tinyurl.com/23tlu5 <-- assert_raise_message

Why the new?

Dont I need to call new to put the strings on the heap? If I just
declare 2 strings on the stack, won't they die when they go out of
scope? The whole point was to put them on the heap so that they hang
around for other threads to process.

Make m_stringvector store strings, and just push them in!

m_stringvector does store strings, and I am pushing strings in (note I
am pushing back '*s1', not 's1').

Sorry - I am probably misunderstanding something, or you are talking
about a concept which is new to me.

Brad

 

[Phlip]

m_stringvector does store strings, and I am pushing strings in (note I
am pushing back '*s1', not 's1').

Didn't notice that, but it's the same difference. Treat objects as objects
for best results. If you truly need threads (you probably don't!), then put
entire high-level objects on the heap. All the data within them should
control themselves. That means the object should store strings, by value.

Sorry - I am probably misunderstanding something, or you are talking
about a concept which is new to me.

Try: Resource Acquisition Is Initialization. It means don't abuse the heap,
and if you do, put the abuse behind clean interfaces that hide the
controlled objects. For example, that's how a std::string itself works.

 

[Jim Langston]

Dont I need to call new to put the strings on the heap? If I just
declare 2 strings on the stack, won't they die when they go out of
scope? The whole point was to put them on the heap so that they hang
around for other threads to process.

m_stringvector does store strings, and I am pushing strings in (note I
am pushing back '*s1', not 's1').

Sorry - I am probably misunderstanding something, or you are talking
about a concept which is new to me.

The std::strings will hang around as long as the vector will. So they won't
go out of scope until the vector does, when the vecotor delete will cause
them to be deleted. Things are put into a std::vector by copy. So the
local std::strings get copied into the vector. The local std::string can go
out of scope, but the vector's copy of them will remain, until it goes out
of scope. In other words, no reason for using new. And in this case new is
a bad thing, because the when the vector's destructor gets called, it will
call the destructors for the std::string points, not the strings themselves,
and you'll be leaking memory unless you manually go through and delete the
elements of the vector.

 

[BobR]

Dont I need to call new to put the strings on the heap? If I just
declare 2 strings on the stack, won't they die when they go out of
scope?

Not if you 'store' them. They will be destroyed at end-of-scope, but, live
on in the 'container' (std::vector below).

#include <iostream>
#include <string>
#include <vector>
#include <iterator>
#include <algorithm> // copy

void VecRefFill( std::vector<std::string> &thevec ){
std::string S1( "Hello" );
std::string S2( "World" );
thevec.push_back( S1 );
thevec.push_back( S2 );
thevec.push_back( "Third string direct." );
return;
} // VecRefFill(vector<string>& )
// at this point, 'S1' and 'S2' are gone, but,
// they were copied into the vector.

std::vector<std::string> VecCopyFill(){
std::vector<std::string> thevec;
std::string S1( "Hello2" );
std::string S2( "World2" );
thevec.push_back( S1 );
thevec.push_back( S2 );
thevec.push_back( "Third string direct2." );
return thevec; // return a 'copy'
}

int main(){
std::vector<std::string> Vec1;
VecRefFill( Vec1 );
std::copy( Vec1.begin(), Vec1.end(),
std:stream_iterator<std::string>( std::cout, "\n" ) );
std::cout<<std::endl;

std::vector<std::string> Vec2;
Vec2 = VecCopyFill();
std::copy( Vec2.begin(), Vec2.end(),
std:stream_iterator<std::string>( std::cout, "\n" ) );

std::string GetItBack( Vec1.at(2) );
// std::cout<<GetItBack<<std::endl;

return 0;
} // main()
// at this point, 'Vec1' and 'Vec2' (and all their strings) are gone.
// all the dynamic memory stuff was handled for you.

/* -output-
Hello
World
Third string direct.

Hello2
World2
Third string direct2.
*/

Did that help any?

--
Bob R
POVrookie
- -
Get "Thinking in C++", 2nd ed. Volume 1&2 by Bruce Eckel
(available for free here. You can buy it in hardcopy too.):
http://www.mindview.net/Books/TICPP/ThinkingInCPP2e.html
Alf P. Steinbach's "Pointers" document:
http://home.no.net/dubjai/win32cpptut/special/pointers/ch_01.pdf
The Dinkumware site has some very good documentation, try:
http://www.dinkumware.com/manuals/.
FAQ http://www.parashift.com/c++-faq-lite

 

[brad.power]

Not if you 'store' them. They will be destroyed at end-of-scope, but, live
on in the 'container' (std::vector below).

#include <iostream>
#include <string>
#include <vector>
#include <iterator>
#include <algorithm> // copy

void VecRefFill( std::vector<std::string> &thevec ){
std::string S1( "Hello" );
std::string S2( "World" );
thevec.push_back( S1 );
thevec.push_back( S2 );
thevec.push_back( "Third string direct." );
return;
} // VecRefFill(vector<string>& )
// at this point, 'S1' and 'S2' are gone, but,
// they were copied into the vector.

std::vector<std::string> VecCopyFill(){
std::vector<std::string> thevec;
std::string S1( "Hello2" );
std::string S2( "World2" );
thevec.push_back( S1 );
thevec.push_back( S2 );
thevec.push_back( "Third string direct2." );
return thevec; // return a 'copy'
}

int main(){
std::vector<std::string> Vec1;
VecRefFill( Vec1 );
std::copy( Vec1.begin(), Vec1.end(),
std:stream_iterator<std::string>( std::cout, "\n" ) );
std::cout<<std::endl;

std::vector<std::string> Vec2;
Vec2 = VecCopyFill();
std::copy( Vec2.begin(), Vec2.end(),
std:stream_iterator<std::string>( std::cout, "\n" ) );

std::string GetItBack( Vec1.at(2) );
// std::cout<<GetItBack<<std::endl;

return 0;
} // main()
// at this point, 'Vec1' and 'Vec2' (and all their strings) are gone.
// all the dynamic memory stuff was handled for you.

/* -output-
Hello
World
Third string direct.

Hello2
World2
Third string direct2.
*/

Did that help any?

--
Bob R
POVrookie
- -
Get "Thinking in C++", 2nd ed. Volume 1&2 by Bruce Eckel
(available for free here. You can buy it in hardcopy too.):
http://www.mindview.net/Books/TICPP/ThinkingInCPP2e.html
Alf P. Steinbach's "Pointers" document:http://home.no.net/dubjai/win32cpptut/special/pointers/ch_01.pdf
The Dinkumware site has some very good documentation, try:
http://www.dinkumware.com/manuals/.
FAQ http://www.parashift.com/c++-faq-lite

Yes - that has helped. I did not know that local objects added to a
vector would live on, which is quite convenient. So I have learned
something from your excellent answers (all posters), which was the
whole point of posting this in the first place.

The only issue now (and possibly a reason to use my implementation
over the ones suggested) is that the API's that I am passing back to
need to take a char*.

std::string s1 = "Hello, World";
call_some_function(s1.c_str()); //Pass a const char* to some api
m_stringvector.push_back(s1);

Now, once s1 goes out of scope, it will destruct. I assume the copy of
s1 sitting in the vector is exactly that - a copy. So the pointer
value of the vectors s1.c_str() would be different to the local
s1.c_str() (even though they would both contain the same string data).
This means that once s1 goes out of scope, my c_str() array pointers
that I have passed to external API's are invalid. Unless I do:

std::string s1 = "Hello, World";
m_stringvector.push_back(s1);
call_some_function((m_stringvector.back()).c_str());

In any case, I guess I just decided to put them on the heap and make
sure objects (and not pointers) were put in the vectors. To be clear,
in case people have missed it:

std::vector<std::string> m_stringvector; //this is private a class
member of SomeClass

....in SomeClass::someMethod()...
std::string* s1 = new std::string();
*s1 = "Hello, World";
call_some_function(s1->c_str());
m_stringvector.push_back(*s1); //Note I am pushing *s1 here, which is
a std::string, not a pointer.


So can I just confirm that there is no memory leak here, and that the
vector will clean up the string object(s) when the class destructs? I
have seen a few people saying that this will leak, but I am not sure
if it is because they have missed the fact that I am actually putting
string objects in the vector (and not pointers), or if there actually
is a memory problem here.

I can see from the suggestions that I don't really need to use new()
here, and I will probably change the implementation, but I just want
to make sure that my existing implementation is not fundamentally
flawed.

Brad

P.S. I tried to avoid threads, but the other api's I am working with
need me to use them, or hang the UI while I do some long task.

 

[Phlip]

std::string s1 = "Hello, World";
call_some_function(s1.c_str()); //Pass a const char* to some api
m_stringvector.push_back(s1);

Now, once s1 goes out of scope, it will destruct. I assume the copy of
s1 sitting in the vector is exactly that - a copy. So the pointer
value of the vectors s1.c_str() would be different to the local
s1.c_str() (even though they would both contain the same string data).
This means that once s1 goes out of scope, my c_str() array pointers
that I have passed to external API's are invalid.

Can you positively determine this? Can you read the API's source, or its
documentation, or its tests?

Can you do this?

char yo[] = "hello world";
call_some_function(yo);
strcpy(yo, "bye-bye");

What string comes out the other end of this API? I don't know about other
programmers, but if I were writing a C API then I would take the Interface
part very seriously, and would not abuse pointers to things my clients
passed in.

P.S. I tried to avoid threads, but the other api's I am working with
need me to use them, or hang the UI while I do some long task.

In this case, you need to learn threading, and then use a semaphore to
ensure your thread-side objects live as long as the thread. And you need to
put your strings into a container (like your vector) which is itself a
member of these thread-side objects. Put the entire object onto the side of
your API - not just each danged string by itself.

And, are you absolutely certain your GUI will hang if you run the API? Does
it have any timer or semaphore system you can use to strobe your GUI?
Sometimes these systems are more robust...

 

[brad.power]

std::string s1 = "Hello, World";
call_some_function(s1.c_str()); //Pass a const char* to some api
m_stringvector.push_back(s1);

Now, once s1 goes out of scope, it will destruct. I assume the copy of
s1 sitting in the vector is exactly that - a copy. So the pointer
value of the vectors s1.c_str() would be different to the local
s1.c_str() (even though they would both contain the same string data).
This means that once s1 goes out of scope, my c_str() array pointers
that I have passed to external API's are invalid.

Can you positively determine this? Can you read the API's source, or its
documentation, or its tests?

Can you do this?

char yo[] = "hello world";
call_some_function(yo);
strcpy(yo, "bye-bye");

What string comes out the other end of this API? I don't know about other
programmers, but if I were writing a C API then I would take the Interface
part very seriously, and would not abuse pointers to things my clients
passed in.

So what you are asking is whether or not the other side of the API
takes a copy of the string or just uses whatever the passed pointer
points to? I have done a test and found that it doesn't make its own
copy - it just passes the pointer around internally. Thats actually
the problem I am trying to solve - I am making long-lived copies of
the data in my code so that the pointers passed will continue to be
valid, and I just wanted to see if my 'use a vector to keep track and
clean up' idea was a good one.

In this case, you need to learn threading, and then use a semaphore to
ensure your thread-side objects live as long as the thread. And you need to
put your strings into a container (like your vector) which is itself a
member of these thread-side objects. Put the entire object onto the side of
your API - not just each danged string by itself.

That is exactly what I am doing. This container is sitting in a thread-
side object which is guaranteed to have the same lifetime as the
thread. The vector is meant to be an automatic clean up system, since
once I kick the thread off I want it to clean up after itself.

And, are you absolutely certain your GUI will hang if you run the API? Does
it have any timer or semaphore system you can use to strobe your GUI?
Sometimes these systems are more robust...

Yep. I tried it without threads and it hung, and then I went to the
documentation, which told me to use threads.

--
Phlip
http://www.oreilly.com/catalog/9780596510657/
"Test Driven Ajax (on Rails)"
assert_xpath, assert_javascript, & assert_ajax

Thanks for your help.

 

[BobR]

[snip]
The only issue now (and possibly a reason to use my implementation
over the ones suggested) is that the API's that I am passing back to
need to take a char*.

std::string s1 = "Hello, World";
call_some_function(s1.c_str()); file://Pass a const char* to some api
m_stringvector.push_back(s1);

Now, once s1 goes out of scope, it will destruct. I assume the copy of
s1 sitting in the vector is exactly that - a copy. So the pointer
value of the vectors s1.c_str() would be different to the local
s1.c_str() (even though they would both contain the same string data).
This means that once s1 goes out of scope, my c_str() array pointers
that I have passed to external API's are invalid. Unless I do:

std::string s1 = "Hello, World";
m_stringvector.push_back(s1);
call_some_function((m_stringvector.back()).c_str());

In any case, I guess I just decided to put them on the heap and make
sure objects (and not pointers) were put in the vectors. To be clear,
in case people have missed it:

std::vector<std::string> m_stringvector; file://this is private a class
member of SomeClass

...in SomeClass::someMethod()...
std::string* s1 = new std::string();
*s1 = "Hello, World";

// std::string *s1 = new std::string( "Hello, World" );

call_some_function(s1->c_str());
m_stringvector.push_back(*s1); file://Note I am pushing *s1 here, which is
a std::string, not a pointer.

So can I just confirm that there is no memory leak here, and that the
vector will clean up the string object(s) when the class destructs? I
have seen a few people saying that this will leak, but I am not sure
if it is because they have missed the fact that I am actually putting
string objects in the vector (and not pointers), or if there actually
is a memory problem here.

You pushed a copy of the string into the vector. The vector will destruct
those. But, what about 's1'? You 'new' it, you 'delete' it.

[ something to look out for ]
If you put pointers into a vector, it will destruct the pointers when the
vector is destructed, BUT, not what they point to. You need to do that
before a vector gets destroyed.

{ // some scope
std::vector<Thing*> VecThing; // assume 'class Thing'.
VecThing.push_back( new Thing );
VecThing.push_back( new Thing );

// use VecThing here

for( std::size_t i(0); i < VecThing.size(); ++i ){
delete VecThing.at(i); // delete the Things
} // for(i)
} // the Thing pointers are destructed at this point.

 

[brad.power]

[snip]
The only issue now (and possibly a reason to use my implementation
over the ones suggested) is that the API's that I am passing back to
need to take a char*.

std::string s1 = "Hello, World";
call_some_function(s1.c_str()); file://Pass a const char* to some api
m_stringvector.push_back(s1);

Now, once s1 goes out of scope, it will destruct. I assume the copy of
s1 sitting in the vector is exactly that - a copy. So the pointer
value of the vectors s1.c_str() would be different to the local
s1.c_str() (even though they would both contain the same string data).
This means that once s1 goes out of scope, my c_str() array pointers
that I have passed to external API's are invalid. Unless I do:

std::string s1 = "Hello, World";
m_stringvector.push_back(s1);
call_some_function((m_stringvector.back()).c_str());

In any case, I guess I just decided to put them on the heap and make
sure objects (and not pointers) were put in the vectors. To be clear,
in case people have missed it:

std::vector<std::string> m_stringvector; file://this is private a class
member of SomeClass

...in SomeClass::someMethod()...
std::string* s1 = new std::string();
*s1 = "Hello, World";

// std::string *s1 = new std::string( "Hello, World" );

call_some_function(s1->c_str());
m_stringvector.push_back(*s1); file://Note I am pushing *s1 here, which is
a std::string, not a pointer.

So can I just confirm that there is no memory leak here, and that the
vector will clean up the string object(s) when the class destructs? I
have seen a few people saying that this will leak, but I am not sure
if it is because they have missed the fact that I am actually putting
string objects in the vector (and not pointers), or if there actually
is a memory problem here.

You pushed a copy of the string into the vector. The vector will destruct
those. But, what about 's1'? You 'new' it, you 'delete' it.

Ok so this might be a problem. If I new something, and then push the
object (dereferenced pointer) into the vector, does it copy the
object? If it does then I have a leak, and it would appear that
pushing local (non new()ed) strings into the vector is a better
approach.

[ something to look out for ]
If you put pointers into a vector, it will destruct the pointers when the
vector is destructed, BUT, not what they point to. You need to do that
before a vector gets destroyed.

Yep I am aware of this, but I am not pushing pointers onto the vector
(it is a vector of string, not a vector of string*). I am pushing what
the pointers point to.

m_stringvector.push_back(*s1);

{ // some scope
std::vector<Thing*> VecThing; // assume 'class Thing'.
VecThing.push_back( new Thing );
VecThing.push_back( new Thing );

// use VecThing here

for( std::size_t i(0); i < VecThing.size(); ++i ){
delete VecThing.at(i); // delete the Things
} // for(i)

} // the Thing pointers are destructed at this point.

Right. So if when you push_back an object into a vector, a *copy* of
that object is made (in seperate storage), then I have a memory leak,
since nothing deletes the initial new(). If this is the case, then it
would be better to push locally declared strings into the vector. I
can test this with a quick test harness.

Thanks all for your comments, I have learned a lot.
Brad

 

[tragomaskhalos]

std::string s1 = "Hello, World";
m_stringvector.push_back(s1);
call_some_function((m_stringvector.back()).c_str());

There's potentially a subtle ticking time bomb here -
if the call_some_function code caches the const char*
you're passing, all will be well *until* your m_stringvector
gets resized (for example by you pushing more strings).
If that happens then its contained strings will all get
moved, which (depending on your std::string implementation)
is going to invalidate that const char* pointer.

In this situation, if you can't predict how many elements
m_stringvector will store, then there are a couple of
alternatives that I can see;
a) use a deque instead of a vector; provided you only
insert at the back (or the front) you won't get an
element-invalidating resize
b) use std::string* instead !

...in SomeClass::someMethod()...
std::string* s1 = new std::string();
*s1 = "Hello, World";
call_some_function(s1->c_str());
m_stringvector.push_back(*s1); //Note I am pushing *s1 here, which is
a std::string, not a pointer.

So can I just confirm that there is no memory leak here, and that the
vector will clean up the string object(s) when the class destructs? I
have seen a few people saying that this will leak, but I am not sure
if it is because they have missed the fact that I am actually putting
string objects in the vector (and not pointers), or if there actually
is a memory problem here.

You do still have a leak here; s1 leaks. m_stringvector will indeed
clear up its contained strings, but you're storing a copy of *s1; s1
itself is still your problem.

HTH

 

[brad.power]

There's potentially a subtle ticking time bomb here -
if the call_some_function code caches the const char*
you're passing, all will be well *until* your m_stringvector
gets resized (for example by you pushing more strings).
If that happens then its contained strings will all get
moved, which (depending on your std::string implementation)
is going to invalidate that const char* pointer.

In this situation, if you can't predict how many elements
m_stringvector will store, then there are a couple of
alternatives that I can see;
a) use a deque instead of a vector; provided you only
insert at the back (or the front) you won't get an
element-invalidating resize
b) use std::string* instead !

Yeah sorry using m_stringvector.back() was just for the purposes of
the example, to show that I had added the string to the vector, and
*then* used the vectored string in the external call, instead of the
other way around. I wouldn't actually do that exact code sequence in
real code. Thanks for pointing it out though - you are quite correct.

You do still have a leak here; s1 leaks. m_stringvector will indeed
clear up its contained strings, but you're storing a copy of *s1; s1
itself is still your problem.

Yep you are right - I verified this myself with a few tests. Looks
like pushing stack-scoped strings onto the vector is the way to go.

HTH

Yes it has helped - I would like to reiterate that everyone's
contribution to the thread has helped me improve the implementation of
this little cleanup idea, so thankyou to everyone who replied.

Brad

 

[BobR]

Ok so this might be a problem. If I new something, and then push the
object (dereferenced pointer) into the vector, does it copy the
object? If it does then I have a leak, and it would appear that
pushing local (non new()ed) strings into the vector is a better
approach.

Rather than re-post a bunch of stuff, you may want to check out this thread:

----- Original Message -----
From: BobR
Newsgroups: comp.lang.c++
Sent: Saturday, June 09, 2007 10:43 PM
Subject: Re: problems of storing dynamically created objects in a vector

[ something to look out for ]
If you put pointers into a vector, it will destruct the pointers when the
vector is destructed, BUT, not what they point to. You need to do that
before a vector gets destroyed.

Yep I am aware of this, but I am not pushing pointers onto the vector
(it is a vector of string, not a vector of string*). I am pushing what
the pointers point to.
m_stringvector.push_back(*s1);
[snip]

Right. So if when you push_back an object into a vector, a *copy* of
that object is made (in seperate storage), then I have a memory leak,
since nothing deletes the initial new(). If this is the case, then it
would be better to push locally declared strings into the vector. I
can test this with a quick test harness.

It looks like you are depending on a pointer to the string not changing
addresses, so, push_back is not good. You need to know how many strings will
be in use and set your vector to an initial size.

std::vector<std::string> vStr( 100, std::string( "Hi" ) );
// now you have 100 strings, all set to "Hi", in the vector.
// or: std::vector<std::string> vStr( 100 ); // 100 empty strings.
vStr.at( 2 ) = "Hello World"; // ok
vStr.push_back( "Goodbye" ); // causes re-allocation
vStr.resize(150); // causes re-allocation
// you can check with 'vStr.capacity()'
// if size exceeds capacity, it re-allocates,
// (possibly/usually) changing it's address.

So, I suggest you dump the 'new', and manage the lifetime and size of the
vector.
Let the std library do the dirty work for you. <G>