Templates, Structs and Invalid Pointers - where did it go wrong

[DaveJ]

Recently I was working on a project where I came across an issue where
the program cored and reported "free(): invalid pointer".
I found a resolution for this, but don't fully understand why the
issue occurred in the first place.

The project uses a simple template class that acts as a buffer.
Initially it has a fixed length, but it's append methods will extend
the size of the buffer if necessary.

Another class defines a struct, that contains this template class as a
member. In one function the struct is created, but when it goes out
of scope the program cores.

Examining the core it appears a call to free in the template classes
destructor caused the core. When I traced through the code with the
debugger I found that after the template class constructor exits, the
address that one of its member's points to changes.

Anyway I've included a cut down and simplified version of the code
below. I found the resolution was to modify the struct so that it
takes a reference to the template class.
------ VarBuf Class <VarBuf.h------
#ifndef __TVARBUF_H__
#define __TVARBUF_H__

#include <stddef.h>
#include <string.h>
#include <stdlib.h>

template<size_t Nclass TVarBuf
{
public:
TVarBuf (const char *initStr);
~TVarBuf ();
private:
char m_Buf[N + 1];
char* m_Str;
size_t m_Capacity;
size_t m_Length;
};
#endif

template<size_t N>
TVarBuf<N>::TVarBuf(const char *initStr) :
m_Str(NULL),
m_Capacity(N),
m_Length(0)
{
m_Str = m_Buf;
m_Str[0] = '\0';
}

template<size_t N>
TVarBuf<N>::~TVarBuf()
{
if(m_Str != m_Buf)
{
free(m_Str);
}
}

---------------------------------

---- Main App <main.cpp-----

1 #include "VarBuf.h"
2 #include "stdio.h"
3 #include "time.h"

4 typedef TVarBuf<6ProfKey; // Create a typedef for our template
class

5 typedef struct
6 {
7 time_t m_CurrTime;
8 int m_Docs;
9 int m_OldDocs;
10 ProfKey m_OldestProfUrl; // this is the template class
11 } Prof;
12
13 int main(int argc, char** argv)
14 {
15 {
16 time_t now = time(NULL);
17 Prof profile = {now, 0, 0, NULL};
18 } // Application cores after it leaves here and the profile
object goes out of scope
19 return 0;
20 }

------------------------------------------

Compiling the above code with the following command
g++ -g -Wall -omain main.cpp

and running the main binary results in:
free(): invalid pointer 0xbfffd66c!

I modified the code slightly so that line 10 of main reads:
-----------------------------------------------
ProfKey& m_OldestProfUrl; //take a reference to ProfKey
-----------------------------------------------
and line 17 now reads
------------------------------------------------
ProfKey key(NULL);
Prof profile = {now, 0, 0, key}
------------------------------------------------

Any explanation of whats going on here would be good.

[Ian Collins]

co******@gmail.com wrote:

>
Saying that you can freely use a deprecated feature until it has been
actually removed in the standard is a overall bad practice. The good
practice is to avoid deprecated features when possible.

gets() anyone?

--
Ian Collins.

[courpron]

On 29 août, 23:36, Ian Collins <[e-mail]:

[e-mail] wrote:

Saying that you can freely use a deprecated feature until it has been
actually removed in the standard is a overall bad practice. The good
practice is to avoid deprecated features when possible.

gets() anyone?

That's a good example of a deprecated feature in C that should be
avoided when possible ; using gets() is bad practice.
Alexandre Courpron.

[Pete Becker]

On 2008-08-29 17:55:31 -0400, co******@gmail.com said:

On 29 août, 23:36, Ian Collins <[e-mail]:

>[e-mail] wrote:

>>Saying that you can freely use a deprecated feature until it has been
actually removed in the standard is a overall bad practice. The good
practice is to avoid deprecated features when possible.

gets() anyone?

That's a good example of a deprecated feature in C that should be
avoided when possible ; using gets() is bad practice.

Not because it's deprecated, however.

--
Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com) Author of "The
Standard C++ Library Extensions: a Tutorial and Reference
(www.petebecker.com/tr1book)

[courpron]

On 30 août, 01:17, Pete Becker <[e-mail]:

On 2008-08-29 17:55:31 -0400, [e-mail] said:

On 29 août, 23:36, Ian Collins <[e-mail]:

[e-mail] wrote:

>Saying that you can freely use a deprecated feature until it has been
actually removed in the standard is a overall bad practice. The good
practice is to avoid deprecated features when possible.

gets() anyone?

That's a good example of a deprecated feature in C that should be
avoided when possible ; using gets() is bad practice.

Not because it's deprecated, however.

Yes, you are right here.

It was bad practice before it was deprecated. This case happens
sometimes for a language construct that, from the experience of the
community, is considered bad practice : it might not be simply removed
from the next standard because it is used in too much programs but it
is qualified as deprecated to "normalize" the fact that it should be
avoided.

Alexandre Courpron.

[Pete Becker]

On 2008-08-29 19:42:55 -0400, co******@gmail.com said:

On 30 août, 01:17, Pete Becker <[e-mail]:

>On 2008-08-29 17:55:31 -0400, [e-mail] said:

>>On 29 août, 23:36, Ian Collins <[e-mail]:
[e-mail] wrote:

>>>>Saying that you can freely use a deprecated feature until it has been
actually removed in the standard is a overall bad practice. The good
practice is to avoid deprecated features when possible.

>>>gets() anyone?

>>That's a good example of a deprecated feature in C that should be
avoided when possible ; using gets() is bad practice.

Not because it's deprecated, however.

Yes, you are right here.

It was bad practice before it was deprecated. This case happens
sometimes for a language construct that, from the experience of the
community, is considered bad practice : it might not be simply removed
from the next standard because it is used in too much programs but it
is qualified as deprecated to "normalize" the fact that it should be
avoided.

Right: it's known to be harmful (although the risk is usually
overstated). But, unfortunately, sometimes things are deprecated
because they don't match someone's idea of good style. Which is why the
label "deprecated" in C++ doesn't mean much.

--
Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com) Author of "The
Standard C++ Library Extensions: a Tutorial and Reference
(www.petebecker.com/tr1book)

[courpron]

On 30 août, 13:45, Pete Becker <[e-mail]:

On 2008-08-29 19:42:55 -0400, [e-mail] said:

On 30 août, 01:17, Pete Becker <[e-mail]:

On 2008-08-29 17:55:31 -0400, [e-mail] said:

>On 29 août, 23:36, Ian Collins <[e-mail]:
[e-mail] wrote:

>>>Saying that you can freely use a deprecated feature until it has been
actually removed in the standard is a overall bad practice. The good
practice is to avoid deprecated features when possible.

>>gets() anyone?

>That's a good example of a deprecated feature in C that should be
avoided when possible ; using gets() is bad practice.

Not because it's deprecated, however.

Yes, you are right here.

It was bad practice before it was deprecated. This case happens
sometimes *for a language construct that, from the experience of the
community, is considered bad practice : it might not be simply removed
from the next standard because it is used in too much programs but it
is qualified as deprecated to "normalize" the fact that it should be
avoided.

Right: it's known to be harmful (although the risk is usually
overstated). But, unfortunately, sometimes things are deprecated
because they don't match someone's idea of good style. Which is why the
label "deprecated" in C++ doesn't mean much.

Well, access declarations, for example, should be avoided, not because
they are harmful, but because they are not very known ; using them in
a program may challenge a number of C++ programmers reading it. And,
in practice, programs using access declarations should be very rare.

Now I can understand that C headers are still used widely, even if it
is a deprecated feature. But I see this as an exception rather than
the rule.
Therefore, saying that "you can freely use C headers" happens to be
right (which I didn't know before this discussion) , but saying that,
"as a general rule, you should avoid deprecated features" is also
right, and is a safe practice IMO. As such, it seems to be coherent to
think about "undeprecating" C headers.

Alexandre Courpron.

[James Kanze]

On Aug 29, 9:34 am, "Alf P. Steinbach" <[e-mail]:

* anon:

Pete Becker wrote:

Pete Becker is secretary of the international C++
standardization committee's library group.

He's also the project editor, which means that he is the person
who has the final responsibility for the actual wording in the
standard.

but I can not understand why are you defending the statement
to use C headers in C++ programs, when there are C++ headers
available

He's not.

He's just countering your misguided views, and extremely
patiently.

He's doing it in Pete Becker style:-). I happen to actually
like his style, even when it's one of my own postings he's
criticizing. But it's largely a matter of personal taste what
style one likes or doesn't.

In practice, the C++ committee, in the C++98 standard, exercised
a fair amount of wishful thinking in its definition of the
headers inherited from C. To the point where no implementation
(or almost none) was compliant. C++0x, recognizing reality, has
relaxed the rules, with the results that there is practically no
real difference between <cxxxxxand <xxxxx.h>. Historically, a
number of people took the C++98 at its word, without realizing
the fact that it was practically impossible to implement in
certain environments (most, in fact), and went overboard in
insisting that everyone use <cxxxxxetc., instead of <xxxxx.h>;
g++ even went so far as to issue a warning for <xxxxx.h(even
though its implementation of <cxxxxxwasn't conform). Others
(including myself), faced with the reality of actual compilers,
recommended sticking with <xxxxx.h>, since at least there, we
more or less knew what we were getting. Both are, of course,
fully standard, and the choice of one form or the other depends
on house rules, etc.

That said, with C++0x there will IMO not be much reason to use
the <xinstead of <x.hheaders: about the only reason would
be where you absolutely want to use a "std::" prefix. That's
because C++0x will acknowledge the current reality that an <x>
header may introduce the names in the global namespace. So
it's then better, in my view, if the code says straight out
that this does or may very well introduce names in the global
namespace, and besides, it's the simplest.

Well, that's always been my point of view:-). But I'm willing
to admit that points of view can vary here (and may depend on
which compilers you have to compile with). In the meantime, the
fact that the standard has actually been changed so that
implementations are conform, and so you more or less know what
you're getting, removes the major motivation I had for not using
<cxxxxx>; in my most recent code, I've adopted the newer style
headers for two reasons: 1) I can use the std:: prefix, e.g. to
distinguish the standard function from a similarly named one in
anonymous namespace, and 2) doing so suppresses the warning from
g++ (which is a compiler I have to support).

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[James Kanze]

On Aug 29, 9:32 pm, [e-mail] wrote:

On 29 août, 19:53, Pete Becker <[e-mail]:
Deprecation is implicitely accompanied with a recommandation.
That is the intention.

And as Pete has explained, in this case, the deprecation was
based on wishful thinking. In practice, your company should
establish some sort of rule for its internal use, based on the
standard, but also on the reality of compiler implementations,
etc. (Based on the standard, the obvious recommendation for
templates is to declare them all export, and put the
implementations in a separate source file, like the authors of
the standard intended. The coding guidelines where I work,
however, clearly say that "export" shall not be used.)

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[James Kanze]

On Aug 29, 8:58 pm, Kai-Uwe Bux <[e-mail]:

[e-mail] wrote:

[snip]
Conversely, non-deprecated features can be assumed to be
normative in the next version,

Even that is not really true. I don't see anything in the
current standard that deprecates the use of nullptr or constexpr
as a user defined symbol in the current standard, but that
doesn't mean that code which uses them won't be broken by the
next version of the standard.

This whole business of "deprecated" seems to be playing word
games to me. In practice, a competent programmer will analyse
the complete situation, and take whatever steps he thinks are
necessary to provide for the future, depending on the
requirements at hand. The fact that the standard declares some
feature deprecated is certainly a significant bit of
information, but it is not, in itself, an absolute; in the case
of the <xxxxx.hheaders, I think that those of us with
experience in implementing libraries tended to take a "wait and
see" attitude, not being convinced that the arguments and
decisions of the committee would actually be transformed into
reality. And time has shown us to be right.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[James Kanze]

On Aug 29, 11:55 pm, [e-mail] wrote:

On 29 août, 23:36, Ian Collins <[e-mail]:

[e-mail] wrote:

Saying that you can freely use a deprecated feature until
it has been actually removed in the standard is a overall
bad practice. The good practice is to avoid deprecated
features when possible.

gets() anyone?

That's a good example of a deprecated feature in C that should
be avoided when possible ; using gets() is bad practice.

You missed his point. It's a misfeature of C, which should be
avoided, despite the fact that it hasn't been deprecated.
Deprecation is just one data point to be taken into
consideration when establishing your rules.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[courpron]

On 31 août, 17:52, James Kanze <[e-mail]:

On Aug 29, 9:32 pm, [e-mail] wrote:

On 29 août, 19:53, Pete Becker <[e-mail]:
Deprecation is implicitely accompanied with a recommandation.
That is the intention.

And as Pete has explained, in this case, the deprecation was
based on wishful thinking. *In practice, your company should
establish some sort of rule for its internal use, based on the
standard, but also on the reality of compiler implementations,
etc.

That can't be false, but I'd say that depends on the language. That
fact concerns more C++ programmers than about any other programming
languages since the current C++ compilers are far from being standard-
compliant. While programming, a SML developer, for example, should
care less about the reality of compilers, and more on the language
specification.

Alexandre Courpron.

[courpron]

On 31 août, 18:12, James Kanze <[e-mail]:

On Aug 29, 11:55 pm, [e-mail] wrote:

On 29 août, 23:36, Ian Collins <[e-mail]:

[e-mail] wrote:
Saying that you can freely use a deprecated feature until
it has been actually removed in the standard is a overall
bad practice. The good practice is to avoid deprecated
features when possible.
gets() anyone?

That's a good example of a deprecated feature in C that should
be avoided when possible ; using gets() is bad practice.

You missed his point. *It's a misfeature of C, which should be
avoided, despite the fact that it hasn't been deprecated.
Deprecation is just one data point to be taken into
consideration when establishing your rules.

The point can be multiple here. It can means :

1) gets() should be avoided despite the fact that it is not
deprecated.

=This is off-topic, that doesn't invalidate the fact that a
deprecated feature should be avoided when possible.
2) gets() is (now) a deprecated feature that should be avoided

=OK, that confirms the "deprecation is also a recommandation"
meaning.
3) gets() went deprecated because it was a harmful feature

=That even more confirms that deprecation is seeing as a
recommandation of not using the deprecated feature.

My simple answer was taking into account all those possible meanings.
Alexandre Courpron.

[kwikius]

Pete Becker wrote:
co******@gmail.com wrote:

On 29 août, 22:35, Pete Becker <[e-mail]:

>On 2008-08-29 16:03:54 -0400, [e-mail] said:

>>When I say that a deprecated feature should be avoided, this is based
on an experience in a wide range of languages with different
programming paradigm, and in all of them "deprecation" has a constant
meaning.

Of course. Actual experience with C++ and the C++ standard and the C++
standards committee doesn't count against your general experience in
other languages.

Relying on his C++ language experience to judge general programming
practices and common term definition is IMHO a bad idea. Using any
functionnal language background seems to be a better choice in that
aspect.

Saying that you can freely use a deprecated feature until it has been
actually removed in the standard is a overall bad practice. The good
practice is to avoid deprecated features when possible.

Alexandre Courpron.

Alexandre Courpron.

On 2008-08-29 19:42:55 -0400, co******@gmail.com said:

>On 30 août, 01:17, Pete Becker <[e-mail]:

>>On 2008-08-29 17:55:31 -0400, [e-mail] said:

On 29 août, 23:36, Ian Collins <[e-mail]:
[e-mail] wrote:

>Saying that you can freely use a deprecated feature until it has been
>actually removed in the standard is a overall bad practice. The good
>practice is to avoid deprecated features when possible.

gets() anyone?

That's a good example of a deprecated feature in C that should be
avoided when possible ; using gets() is bad practice.

Not because it's deprecated, however.

Yes, you are right here.

It was bad practice before it was deprecated. This case happens
sometimes for a language construct that, from the experience of the
community, is considered bad practice : it might not be simply removed
from the next standard because it is used in too much programs but it
is qualified as deprecated to "normalize" the fact that it should be
avoided.

Right: it's known to be harmful (although the risk is usually
overstated). But, unfortunately, sometimes things are deprecated because
they don't match someone's idea of good style. Which is why the label
"deprecated" in C++ doesn't mean much.

Just out of interest, how many deprecated features of C++ have been
removed ? None AFAIK

regards
Andy Little

[Pete Becker]

On 2008-08-31 19:55:38 -0400, kwikius <an**@servocomm.freeserve.co.uksaid:

>
Just out of interest, how many deprecated features of C++ have been
removed ? None AFAIK

Exactly right. None, in two revisions.

--
Pete
Roundhouse Consulting, Ltd. (www.versatilecoding.com) Author of "The
Standard C++ Library Extensions: a Tutorial and Reference
(www.petebecker.com/tr1book)

[Old Wolf]

On Aug 29, 1:59*am, [e-mail] wrote:

>
You may be seeing a g++ bug that was fixed at some version but then
resurfaced at a later version. We had a similar issue when we
initialized arrays of such self-referencing types. g++ would
incorrectly do a memberwise copy as if the type did not have a user-
specified copy constructor:

But a member-wise copy is the mandated behaviour
for compiler-generated copy constructors, which is
what you get if you don't specify one.

SelfRef a[] = { SelfRef("one"), SelfRef("two") };

The arrays would be initialized with incorrectly-copied copies of the
temporaries. As a result, the objects' members would not be pointing
to their own members.

This would be a bug in the code, rather than a bug
in the compiler; if I am reading the C++ standard
correctly, then a[0] and a[1] are copy-constructed
from the two temporaries in the brace list.

[anon]

Default User wrote:

anon wrote:

>Default User wrote:

>>anon wrote:

DaveJ wrote:

if(m_Str != m_Buf)
should be
if (m_Str != &m_Buf[0])

Why?

m_Buf is an array

So? What happens when the name of an array is used in most contexts,
outside of the sizeof or address-of (&) operators?

Isn't it an undefined behavior?

[courpron]

On 1 sep, 01:55, kwikius <[e-mail]:

>
Just out of interest, how many deprecated features of C++ have been
removed ? None AFAIK

Indeed. However, a deprecated feature may imply that the feature is
just scarecely use and then should be avoided to follow common
practices (we saw, however, that it was not the case with C headers).

I analyze the fact that deprecated features are not (currently)
removed, with the following 2 points :

1) C++ is a well-established language that is widely use for years
2) One of the goal of the C++ standard commitee is to provide backward
compatibility

The funny thing is that the new C++0x headers (amongst a few other
things) will break backward compatibility (at least in theory, less in
practice), while one reason to maintain C headers as a deprecated
feature was to provide backward compatibility with C.
Alexandre Courpron.

[Default User]

anon wrote:

Default User wrote:

anon wrote:

Default User wrote:
anon wrote:

DaveJ wrote:

if(m_Str != m_Buf)
should be
if (m_Str != &m_Buf[0])

Why?

m_Buf is an array

So? What happens when the name of an array is used in most contexts,
outside of the sizeof or address-of (&) operators?

Isn't it an undefined behavior?

Isn't what undefined behavior? To use the name of an array? No.

When the name of an array is used in most contexts, such as the right
side of an assignment or with the [] operator, or in a comparison, it
is converted to a pointer to the first element. Yhat's both value and
type.


Brian

[James Kanze]

On Aug 31, 6:18 pm, "Alf P. Steinbach" <[e-mail]:

* James Kanze:

in my most recent code, I've adopted the newer style
headers for two reasons: 1) I can use the std:: prefix, e.g. to
distinguish the standard function from a similarly named one in
anonymous namespace, and 2) doing so suppresses the warning from
g++ (which is a compiler I have to support).

Uh, which warning?

I'm aware of warning for pre-standard iostreams, but that's all.

I'm not sure, actually:-). I know that in the past, when I
regularly used "deprecated" headers, I got warnings from g++.
Maybe they were only for strstream; I'm not really sure. But
since I've switched to not using the deprecated headers, they've
gone away. (For all I know, it might just be that g++ fixed
this stupidity---I've also upgraded the compiler since then.)

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]