why not use naked delete ?

[arnuld]

After 5 years of working purely in C, finally I becawe jobless and have
some time on my hands to start C++. Well, I kept on peeking into
stroustrup from time to time but now I see with C++14 everything has
changed. Even in C++, I will allocate memory using /new/ and then free it
using /delete/ but now I see Stroustrup does not seem to agree to that:

http://isocpp.org/blog/2014/12/myths-2

He is calling it naked delete. I know there lot of other examples he has
given but those are using generic/oo programming(half of that code is
incomprehensible to me because IU just started C++ today). I am more
concerned with simple procedural subset of C++:

X* p = new X;
X* q = p;
delete p;
// ...
q->do_something(); // the memory that held *p may have been re-used

STROUSTRUP: Don’t do that. Naked deletes are dangerous

ARNULD: then what I should do ?

Can someone point me to some direction in understanding this ?



--
arnuld
http://lispmachine.wordpress.com/

[Dombo]

Op 26-Dec-14 11:58, arnuld schreef:

Look for the RAII (Resource Acquisition Is Initialization) idiom which
every C++ programmer should be very familiar with. Examples of RAII are
the smart pointer classes std::unique_ptr and std::shared_ptr which
automatically take care of delete at the right time so you don't have to.

The problem with "naked" delete is that it is very easy to leak memory,
especially when you use exceptions. The RAII idiom and smart pointers
makes it much easier to write clean and robust code that doesn't leak
resources. Also when you use container classes like std::vector smart
pointers make life a lot easier.

[Bo Persson]

Another part of the advice is not to use naked pointers either. :-)

Stroustrup shows how to use a unique_ptr when you need a single pointer
to an object. There is also a shared_ptr for when you want to share
ownership of the object. In both cases, the object is automagically
deleted when the last pointer goes away.

You could do:

auto p = std::make_shared<X>();
auto q = p;

// do something that invalidates p

q->do_something(); // q is still valid here



Bo Persson

[Jorgen Grahn]

On Fri, 2014-12-26, arnuld wrote:
> After 5 years of working purely in C, finally I becawe jobless and have
> some time on my hands to start C++. Well, I kept on peeking into
> stroustrup from time to time but now I see with C++14 everything has
> changed. Even in C++, I will allocate memory using /new/ and then free it
> using /delete/ but now I see Stroustrup does not seem to agree to that:
>
> http://isocpp.org/blog/2014/12/myths-2
>
> He is calling it naked delete. I know there lot of other examples he has
> given but those are using generic/oo programming(half of that code is
> incomprehensible to me because IU just started C++ today). I am more
> concerned with simple procedural subset of C++:

If you ignore classes in C++, then you'll have problems. Try instead
to ignore your C knowledge and follow the book (I get the impression
you own "The C++ Programming Language").

> X* p = new X;
> X* q = p;
> delete p;
> // ...
> q->do_something(); // the memory that held *p may have been re-used
>

> STROUSTRUP: Don???t do that. Naked deletes are dangerous

>
> ARNULD: then what I should do ?

But /in the very next sentence/ he explains it! You just have to read
for five seconds more:

Don't do that. Naked deletes are dangerous - and unnecessary in
general/user code. Leave deletes inside resource management
classes, such as string, ostream, thread, unique_ptr, and
shared_ptr. There, deletes are carefully matched with news and
harmless.

Personally I find that 99% of my resource management is done without
any pointers or smart pointers. If I have some object 'foo' it's
either:

- allocated "on the stack" (or whatever the proper term is)
- a member of some bigger object
- an element of some container e.g. a std::vector

No doubt people need new/delete and smart pointers from time to time,
and some people need it frequently -- but you can do /a lot/ of
programming without having to care about such things.

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .

[Mr Flibble]

On 26/12/2014 21:00, Jorgen Grahn wrote:

> - allocated "on the stack" (or whatever the proper term is)

I believe the phrase "on the stack" appears in the ISO C++ standard so I
wouldn't worry about using it; if it didn't appear in the ISO C++
standard I still wouldn't worry about using it.

/Flibble

[Richard]

[Please do not mail me a copy of your followup]

arnuld <sun...@invalid.address> spake the secret code
<549d3f6c$0$280$1472...@news.sunsite.dk> thusly:

>After 5 years of working purely in C, finally I becawe jobless and have
>some time on my hands to start C++. Well, I kept on peeking into
>stroustrup from time to time but now I see with C++14 everything has
>changed.

C++14 hardly changes "everything". You can view it mostly as a "minor
enhancement" to C++11.

If by "changes everything" you mean the changes introduced by C++11/14
compared to C++98, I'd still disagree with that statement. As far as
the core language is concerned, C++11/14 make it easier and less
problematic to write C++ code than C++98. Changes (as opposed to
additions) to the standard library weren't that significant IMO.

>Even in C++, I will allocate memory using /new/ and then free it
>using /delete/ but now I see Stroustrup does not seem to agree to that:

The only time new or delete (almost the lowest-level mechanism for
managing memory) should appear in your code is in a class designed to
manage the resource acquired by new and released by delete. Since the
standard library already provides two resource managing clases for
this purpose, std::unique_ptr and std::shared_ptr, there isn't any
reason you should be using new or delete directly in your code.

Furthermore, since most uses of new/delete have to do with creating
blocks of memory to hold collections of values and the standard
library provides container classes already, there is even less of a
need to use new/delete directly.

Need a variable-length buffer of characters to pass to some C style
function? Use std::vector<char>.

Need a string? Use std::string.

Need a dynamically resizable array of some other type T? Use
std::vector<T>.

Need a dictionary to look up values of type T from strings? Use
std::map<std::string, T>.

>[...] I am more

>concerned with simple procedural subset of C++:
>
> X* p = new X;
> X* q = p;

Well, this is poor form already. Who "owns" the instance of X? p or q?

If you had written:

X* p = new X;

X &q = *p;
// ...
delete p;
q.do_something(); // oops, still a problem.

Then your code would be revealing your intention instead of confusing
the reader. If we use scoping to limit the use of q to only where it
is intended, then we solve the "oops"

X* p = new X;

{
X &q = *p;
// ...
q.do_something();
}
delete p;

Now we've used scoping to explicitly reveal that q has a dependency on
p and that p's lifetime should extend beyond the lifetime of q.
However, we still have a problem if do_something() or anything else in
that block can throw an exception. If an exception is thrown, the
stack is unwound and we've leaked the memory pointed to by p.

I know this is only code you've written for the purposes of
discussion, but let's talk about another thing shown here. Is the use
of the heap really necessary? Why not write:

X p;
X &q = p;

...and then we don't have to worry about whether or not q can still be
used -- its lifetime is guaranteed to be shorter than p's lifetime.
We also don't need to worry about who is responsible for destroying p;
it's destroyed when we leave the scope of p. We also get code that is
exception safe.

We shouldn't use the heap unless it is really necessary.

So how do you get exception-safe use of the heap? First, make sure
you really need the heap (your toy example does not). Second, if you
really must use the heap, consider what you are trying to do with
dynamically allocated memory.

Are you trying to make a container? Use one of the standard library
container classes for that.

Are you trying to extend the lifetime of some data beyond the scope
of the code that creates it? Try std::unique_ptr.

Are you really trying to create shared data structures? Try
std::shared_ptr. (Note: shared data structures can lead to all kinds
of problems and I don't recommend blindly throwing shared_ptr at
everything. Most applications have very few required uses of shared
data, where the data must really be shared.

Use <http://cppreference.com> as your reference to the standard
library classes if you don't have a good book. I like Nicolai
Josuttis "The C++ Standard Library" myself.
--
"The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeline>
The Computer Graphics Museum <http://computergraphicsmuseum.org>
The Terminals Wiki <http://terminals.classiccmp.org>
Legalize Adulthood! (my blog) <http://legalizeadulthood.wordpress.com>

[Richard]

[Please do not mail me a copy of your followup]

Jorgen Grahn <grahn...@snipabacken.se> spake the secret code
<slrnm9rj2v.1...@frailea.sa.invalid> thusly:

>Personally I find that 99% of my resource management is done without
>any pointers or smart pointers.

Bingo. For me, it's more 9s than that :-), more like 99.99999%.

Once you start using the facilities that C++ has been giving you for
over a decade, all those stupid memory leak and pointer bugs just
disappear. Every time I hear someone complaining about memory leaks
or pointer bugs in C++, they keep showing me C code compiled with a
C++ compiler.

Please, just stop writing C code in C++!

[Lynn McGuire]

We use new and delete a lot in our base classes since we are memory constrained in the Win32 environment. We rarely use new or
delete in the descendent classes since they are so powerful.

Lynn

[Luca Risolia]

Il 29/12/2014 18:37, Lynn McGuire ha scritto:

> We use new and delete a lot in our base classes since we are memory
> constrained in the Win32 environment.

std::unique_ptr has been designed so that it can take the same size as a
pointer by default, isn't this the case for Win32?

[Richard]

[Please do not mail me a copy of your followup]

Lynn McGuire <l...@winsim.com> spake the secret code
<m7s3fr$df9$1...@dont-email.me> thusly:

>We use new and delete a lot in our base classes since we are memory
>constrained in the Win32 environment.

By "memory constrained" am I to infer you are talking about stack
space?

Why naked new and not std::unique_ptr<> or some sort of container?

[Lynn McGuire]

On 12/29/2014 10:29 PM, Richard wrote:
> [Please do not mail me a copy of your followup]
>
> Lynn McGuire <l...@winsim.com> spake the secret code
> <m7s3fr$df9$1...@dont-email.me> thusly:
>
>> We use new and delete a lot in our base classes since we are memory
>> constrained in the Win32 environment.
>
> By "memory constrained" am I to infer you are talking about stack
> space?

We run out of Win32 space if we are not careful. Probably heap space since some of our datasets go over one GB. Things are better
not though since we started compressing strings in memory.

> Why naked new and not std::unique_ptr<> or some sort of container?

This code was written over a decade ago and works well. Never rewrite code that is working well just to use new coding features.

Lynn

[Öö Tiib]

There is difference between writing:
"We used new and delete a lot in our base classes that we wrote over
decade ago."
Or:

"We use new and delete a lot in our base classes since we are memory
constrained in the Win32 environment."

First is common, lot of people did it over decade ago. Second is
intriguing because writing naked 'new' and 'delete' in modern C++
adds complexities (and so defects) but does not save resources.

[woodb...@gmail.com]

I disagree with that advice, but in this case you might
be better off sticking with new and delete.

Brian
Ebenezer Enterprises - In G-d we trust.
http://webEbenezer.net

[Lynn McGuire]

So how would you implement this class without using new? Here is the class and constructor using a reference value:

class DesValue : public ObjPtr
{
public:

int datatype; // Either #Int, #Real, or #String.
int vectorFlag; // Flag indicating value contains an Array.
int optionListName; // name of the optin list item
int * intValue; // Either nil, an Int, a Real, a String, or an Array thereof.
double * doubleValue;
string * stringValue;
vector <int> * intArrayValue;
vector <double> * doubleArrayValue;
vector <string> * stringArrayValue;
unsigned char * compressedData;
unsigned long compressedDataLength;
vector <unsigned long> uncompressedStringLengths;
std::string * uncompressedString;
int isTouched; // Flag indicating if value, stringValue, or units have been modified since this DesValue was created. Set to
true by setValue, setString, setUnits, and convertUnits.
int isSetFlag; // Flag indicating whether the contents of the DesValue is defined or undefined. If isSet is false, getValue
returns nil despite the contents of value, while getString and getUnits return the empty string despite the contents of stringValue
and units.
int unitsValue; // current string value index in $UnitsList (single or top)
int unitsValue2; // current string value index in $UnitsList (bottom)
string errorMessage; // message about last conversion of string to value
string unitsArgs; // a coded string of disallowed units

public:

// constructor
DesValue ();
DesValue (const DesValue & rhs);
DesValue & operator = (const DesValue & rhs);

// destructor
virtual ~DesValue ();

virtual DesValue * clone () { return new DesValue ( * this); }
...
}

DesValue::DesValue (const DesValue & rhs)
{
datatype = rhs.datatype;
vectorFlag = rhs.vectorFlag;
optionListName = rhs.optionListName;
if (rhs.intValue)
{
intValue = new int;
* intValue = * rhs.intValue;
}
else
intValue = NULL;
if (rhs.doubleValue)
{
doubleValue = new double;
* doubleValue = * rhs.doubleValue;
}
else
doubleValue = NULL;
if (rhs.stringValue)
{
try
{
stringValue = new string;
* stringValue = * rhs.stringValue;
}
catch (std::bad_alloc &ba)
{
char msg [1024];
sprintf_s (msg, sizeof (msg),
"A memory error has occurred that could not be handled.\n"
"Please try the operation again.\n\n"
"Message: %s\n"
"Size: %d bytes", ba.what (), rhs.stringValue -> size ());
alert (msg);
}
}
else
stringValue = NULL;
if (rhs.intArrayValue)
{
intArrayValue = new vector <int>;
* intArrayValue = * rhs.intArrayValue;
}
else
intArrayValue = NULL;
if (rhs.doubleArrayValue)
{
doubleArrayValue = new vector <double>;
* doubleArrayValue = * rhs.doubleArrayValue;
}
else
doubleArrayValue = NULL;
if (rhs.stringArrayValue)
{
stringArrayValue = new vector <string>;
* stringArrayValue = * rhs.stringArrayValue;
}
else
stringArrayValue = NULL;

if (rhs.compressedData && rhs.compressedDataLength)
{
unsigned long num = rhs.uncompressedStringLengths.size ();
if (vectorFlag && num)
{
// if a vector of strings, copy the uncompressed string lengths
uncompressedStringLengths.resize (num);

for (unsigned long i = 0; i < num; i++)
uncompressedStringLengths [i] = rhs.uncompressedStringLengths [i];
}

// copy the size of the compressed data
compressedDataLength = rhs.compressedDataLength;

// allocate and copy the compressed data
compressedData = (unsigned char *) malloc (rhs.compressedDataLength);
if ( ! compressedData)
alert ("DesValue::DesValue (const DesValue &) - unable to malloc " +
asString (rhs.compressedDataLength) + " bytes");
memcpy_s (compressedData, rhs.compressedDataLength,
rhs.compressedData, rhs.compressedDataLength);
}
else
{
compressedData = NULL;
compressedDataLength = 0;
uncompressedStringLengths.resize (0);
}

if (rhs.uncompressedString)
{
uncompressedString = new std::string;
* uncompressedString = * rhs.uncompressedString;
}
else
uncompressedString = NULL;

isTouched = rhs.isTouched;
isSetFlag = rhs.isSetFlag;
unitsValue = rhs.unitsValue;
unitsValue2 = rhs.unitsValue2;
errorMessage = rhs.errorMessage;
unitsArgs = rhs.unitsArgs;
}

Lynn

[Chris Vine]

Has it? Every object of class type must have a size of at least one.
§20.7.1.2.4/7 of the C++11 standard suggests that an object of the
deleter's type is stored as a class member rather than constructed on
the fly when it is called (get_deleter() returns "a reference to the
stored deleter"). This means that a std::unique_ptr object has a size
comprising the size of a pointer + 1, assuming the deleter is a
functor class without data members, as would usually be the case (it
is also permitted by the standard be a function pointer, or more
correctly an lvalue reference to function, which would normally have a
larger size).

Of course, returning a reference to the deleter means that the standard
might as well have made it a public member, but that is a separate
issue.

Chris

[Richard]

[Please do not mail me a copy of your followup]

Lynn McGuire <l...@winsim.com> spake the secret code

<m7v5kd$87i$1...@dont-email.me> thusly:

>On 12/29/2014 10:29 PM, Richard wrote:
>> By "memory constrained" am I to infer you are talking about stack
>> space?
>
>We run out of Win32 space if we are not careful. Probably heap space
>since some of our datasets go over one GB. Things are better
>not though since we started compressing strings in memory.

Very large datasets are almost certainly going to be allocated on the
heap and yeah, 32-bit Windows programs have the weird 2 GB/4 GB
process space limitations that have haunted games for some time.

>> Why naked new and not std::unique_ptr<> or some sort of container?
>
>This code was written over a decade ago and works well. Never rewrite
>code that is working well just to use new coding features.

OK, but presumably for new code you are using std::unique_ptr?

[Chris Vine]

Having said that, on testing I see that std::unique_ptr on my
implementation has the size of a pointer. On looking at the
implementation, instead of having separate pointer and deleter data
members, it has a tuple member comprising the pointer and the deleter,
so eliminating the need to give the deleter a size of 1 because tuples
are implemented by recursive inheritance. Quite clever.

Chris

[Öö Tiib]

I do not know the purpose of it. So I can't tell for sure what I would do.
Feels it has a lot of members so possibly I would consider splitting
it up or reducing members somehow. I will comment below:

> class DesValue : public ObjPtr
> {
> public:
>
> int datatype; // Either #Int, #Real, or #String.
> int vectorFlag; // Flag indicating value contains an Array.
> int optionListName; // name of the optin list item
> int * intValue; // Either nil, an Int, a Real, a String, or an Array thereof.
> double * doubleValue;
> string * stringValue;
> vector <int> * intArrayValue;
> vector <double> * doubleArrayValue;
> vector <string> * stringArrayValue;

Above part of it seems to be a variant. If it is so then instead I would
use some existing implementation of variant. For example boost::variant:

typedef boost::variant< nullptr_t,
int,
double,
std::string,
std::vector<int>,
std::vector<double>,
std::vector<std::string>
> ValueVariant;
ValueVariant variantValue;

Its copy-construction is default.

The 'datatype' can be changed to function '{return variantValue.which();}'.
The 'vectorFlag' can be changed to function '{return datatype() > 3;}''.

> unsigned char * compressedData;
> unsigned long compressedDataLength;

Why not vector instead of above? Like:

std::vector<unsigned char> compressedData;

Its copy-construction is default.

> vector <unsigned long> uncompressedStringLengths;
> std::string * uncompressedString;

Why above is pointer? I would replace it with just 'std::string':

std::string uncompressedString;

That would also copy by default. As are rest of the members.

> int isTouched; // Flag indicating if value, stringValue, or units have been modified since this DesValue was created. Set to
> true by setValue, setString, setUnits, and convertUnits.
> int isSetFlag; // Flag indicating whether the contents of the DesValue is defined or undefined. If isSet is false, getValue
> returns nil despite the contents of value, while getString and getUnits return the empty string despite the contents of stringValue
> and units.
> int unitsValue; // current string value index in $UnitsList (single or top)
> int unitsValue2; // current string value index in $UnitsList (bottom)
> string errorMessage; // message about last conversion of string to value
> string unitsArgs; // a coded string of disallowed units
>
> public:
>
> // constructor
> DesValue ();
> DesValue (const DesValue & rhs);

Above copy constructor could be just defaulted if to do like I suggested
above since compiler-generated is as good:

DesValue (const DesValue & rhs) = default;

> DesValue & operator = (const DesValue & rhs);
>
> // destructor
> virtual ~DesValue ();

Likely copy assignment and destructor and move constructor and move
assignment as well.

>
> virtual DesValue * clone () { return new DesValue ( * this); }

Yes, that has to remain if you need virtual clone with covariance
because there are no covariance between 'std::unique_ptr<base>' and
'std::unique_ptr<derived>'.

> ...
> }

Looks quite big snip:

[woodb...@gmail.com]

On Tuesday, December 30, 2014 6:28:57 PM UTC-6, Richard wrote:
> [Please do not mail me a copy of your followup]
>
> Lynn McGuire <l...@winsim.com> spake the secret code
> <m7v5kd$87i$1...@dont-email.me> thusly:
>
> >On 12/29/2014 10:29 PM, Richard wrote:
> >> By "memory constrained" am I to infer you are talking about stack
> >> space?
> >
> >We run out of Win32 space if we are not careful. Probably heap space
> >since some of our datasets go over one GB. Things are better
> >not though since we started compressing strings in memory.
>
> Very large datasets are almost certainly going to be allocated on the
> heap and yeah, 32-bit Windows programs have the weird 2 GB/4 GB
> process space limitations that have haunted games for some time.
>
> >> Why naked new and not std::unique_ptr<> or some sort of container?
> >
> >This code was written over a decade ago and works well. Never rewrite
> >code that is working well just to use new coding features.
>
> OK, but presumably for new code you are using std::unique_ptr?

Sometimes I use unique_ptr but other times new and delete
in new code. I think it's good to keep your options open.

Brian
Ebenezer Enterprises -
http://webEbenezer.net


Brian
Ebenezer Enterprises

[Öö Tiib]

Yes, the deleter is like static member of class of 'unique_ptr'.
The 'shared_ptr'/'weak_ptr' family is kind of heavyweight but
since shared ownership is sort of rare optimization itself (lets
share instead of making copies) it does not matter much in practice.