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.

DaveJ wrote:
[...]

>
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.
>

It doesn't core for me (unfortunately)

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

Here is even smaller example producing exactly what you are doing:

int main()
{
int a = 5;
int *b = &a;
free( b );
}

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.
>

I guess you are out of luck. What you are doing is not allowed.

>
------ VarBuf Class <VarBuf.h------
#ifndef __TVARBUF_H__
#define __TVARBUF_H__
>
#include <stddef.h>
#include <string.h>
#include <stdlib.h>

These are c headers. Should be:
#include <cstddef>
#include <cstring>
#include <cstdlib>

>
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) :

initStr is not used

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)

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

{
free(m_Str);

Where do you allocate this memory?

}
}
>
---------------------------------
>
---- Main App <main.cpp-----
>
1 #include "VarBuf.h"
2 #include "stdio.h"
3 #include "time.h"

Do not use c headers in a c++ program

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

This typedef looks very C-ish

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}
------------------------------------------------

This doesn't solve your problem, which is in the TVarBuf class

On Aug 28, 10:36 am, anon <a...@no.nowrote:

DaveJ wrote:
>
initStr is not used

I've reduced the size of VarBuf.h as the actual class is fairly
large. The real class does use the initStr, but in my case it should
always be NULL.

I think the issue lies around the re-assignment of m_Str.

In the constructor m_Str = m_Buf; seems fine.
After the constructor exists the address that m_Str points to suddenly
changes.

This only occurs if I create the VarBuf from within a struct (with or
without the typedef). If I create it directly in the method there is
no problem.

It's interesting that you say it didn't core for you. Initially this
was in a program which ran on RedHat3 for years without issue. When
we moved it to RH5 the core started occurring (although I have now
been able to replicate it on some RH3 machines).

DaveJ wrote:

should be:
m_Str = &m_Buf[0];

>
I think the issue lies around the re-assignment of m_Str.
>
In the constructor m_Str = m_Buf; seems fine.
After the constructor exists the address that m_Str points to suddenly
changes.
>

Sorry I missed that somehow. Anyway, like that it should work fine.

>

>
This only occurs if I create the VarBuf from within a struct (with or
without the typedef). If I create it directly in the method there is
no problem.
>
It's interesting that you say it didn't core for you. Initially this
was in a program which ran on RedHat3 for years without issue. When
we moved it to RH5 the core started occurring (although I have now
been able to replicate it on some RH3 machines).
>

RH3?? wow what compiler are you using?
I compiled your example on RH8, with gcc 4.1.3 and it works fine.

Try running your real program (or your example) with valgrind and see
what is the problem

On 2008-08-28 05:36:48 -0400, anon <anon@no.nosaid:

>
These are c headers. Should be:

They're also C++ headers.

#include <cstddef>
#include <cstring>
#include <cstdlib>

>
Do not use c headers in a c++ program

What problems do you think this will cause? Other than requiring the
addition of lots of std:: qualifiers, that is.

>

>
This typedef looks very C-ish

Hmm, I didn't know that C had templates. <gSeriously, typedefs can
simplify code. What, specifically, do you object to here?

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

On 2008-08-28 04:35:13 -0400, DaveJ <davej2000@gmail.comsaid:

>
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;
};

Just guessing, but the problem probably comes up because this template
doesn't have a copy constructor and assignment operator. When an object
of this type gets copied (by the default copy operations), the new
address of m_Buf is different from the old one, but m_Str still points
to the old one. So the destructor for the new one will see that they're
different, and try to delete m_Str.

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

On Aug 28, 12:25*pm, Pete Becker <p...@versatilecoding.comwrote:

On 2008-08-28 04:35:13 -0400, DaveJ <davej2...@gmail.comsaid:
>
>
>

>
Just guessing, but the problem probably comes up because this template
doesn't have a copy constructor and assignment operator. When an object
of this type gets copied (by the default copy operations), the new
address of m_Buf is different from the old one, but m_Str still points
to the old one. So the destructor for the new one will see that they're
different, and try to delete m_Str.
>

It's a good suggestion, I had left this out of the original post, but
the code does
already have a copy constructor
template<size_t N>
TVarBuf<N>::TVarBuf(const TVarBuf<N&that) :
m_Capacity(that.m_Capacity),
m_Length(that.m_Length)
{
printf("Im called\n");
m_Str = (that.m_Str == that.m_Buf) ? m_Buf
: (char *) malloc(m_Capacity +
1);

// Copy including the terminating NULL.
memcpy(m_Str, that.m_Str, m_Length + 1);
}

Pete Becker wrote:

On 2008-08-28 05:36:48 -0400, anon <anon@no.nosaid:
>

>
They're also C++ headers.
>

>

>
What problems do you think this will cause? Other than requiring the
addition of lots of std:: qualifiers, that is.
>

Once you start including stdio.h, time.h, etc. you might start including
iostream.h

>
Hmm, I didn't know that C had templates. <gSeriously, typedefs can
simplify code. What, specifically, do you object to here?
>

I suggested to change this:

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;

into this:

5 struct Prof
6 {
7 time_t m_CurrTime;
8 int m_Docs;
9 int m_OldDocs;
10 ProfKey m_OldestProfUrl; // this is the template class
11 };

not that is going to fix whatever problem he is experiencing

On 2008-08-28 07:42:46 -0400, anon <anon@no.nosaid:

Pete Becker wrote:

>
Once you start including stdio.h, time.h, etc. you might start
including iostream.h

But your objection was to using C headers, not to using non-standard headers.

>

>
I suggested to change this:
>
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;
>
into this:
>
5 struct Prof
6 {
7 time_t m_CurrTime;
8 int m_Docs;
9 int m_OldDocs;
10 ProfKey m_OldestProfUrl; // this is the template class
11 };
>
not that is going to fix whatever problem he is experiencing

That still uses the typedef that you objected to. I agree that the
typedef that you changed in this code isn't appropriate in C++.

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

On 2008-08-28 07:40:55 -0400, DaveJ <davej2000@gmail.comsaid:

On Aug 28, 12:25Â*pm, Pete Becker <p...@versatilecoding.comwrote:

>
It's a good suggestion, I had left this out of the original post, but
the code does
already have a copy constructor

What else did you leave out that's critical to diagnosing the problem?
(Don't answer: that's rhetorical)

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

On 28 elo, 14:40, DaveJ <davej2...@gmail.comwrote:

It's a good suggestion

I have a better one: Don't use malloc/free, use new and delete.

Pete Becker wrote:

On 2008-08-28 07:42:46 -0400, anon <anon@no.nosaid:
>

>
But your objection was to using C headers, not to using non-standard
headers.
>

C headers in c++ program

>
That still uses the typedef that you objected to. I agree that the
typedef that you changed in this code isn't appropriate in C++.
>

I guess both issues are matter of personal preferences. You can program
like this if you like: http://www.ioccc.org/2004/anonymous.c and it will
still be a valid c++ program


I can not reproduce the core dump he is getting, therefore it can be the
compiler he is using.

On Aug 28, 12:52*pm, Pete Becker <p...@versatilecoding.comwrote:

On 2008-08-28 07:40:55 -0400, DaveJ <davej2...@gmail.comsaid:
>
>
>

>

>

>

>
What else did you leave out that's critical to diagnosing the problem?
(Don't answer: that's rhetorical)

I thought it was best to leave out the copy constructor as it's not
actually called in this occurrence of the program.
Infact I stripped out everything that is not used, so that only the
critical bits of code can be examined.

On Aug 28, 4:40*am, DaveJ <davej2...@gmail.comwrote:

[...]

It's a good suggestion, I had left this out of the original post, but
the code does
already have a copy constructor

I assume that you took care of the assignment as well.

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:

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.

Ali

On 2008-08-28 08:47:33 -0400, anon <anon@no.nosaid:

Pete Becker wrote:

>
C headers in c++ program

Huh? Of course we were talking about C++ programs. The standard C
headers, slightly modified, are standard headers in C++, too.

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

On Aug 28, 3:35 am, DaveJ <davej2...@gmail.comwrote:

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.

...

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);
}
>
}

Destructor frees invalid pointer eventually if object was created by
memmove, default-copy, or default-assignment. Evidently you already
noticed this.

---------------------------------
>
---- 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;

This should be an aggregate non-POD-struct, so there is no reason to
try to be both C and C++. It would be typical style both to rewrite
as a non-typedef'd struct as mentioned in an earlier post, and use the
C++-specific standard headers.

You'll miss some archaic C++ compiler bugs by sacrificing aggregate
initialization and rewriting (modulo style guidelines) as

class Prof
{
public:
time_t m_CurrTime;
int m_Docs;
int m_OldDocs;
ProfKey m_OldestProfUrl; // this is the template class

Prof(time_t _m_CurrTime, int _m_Docs, int _m_OldDocs, const char*
InitStr)
: m_CurrTime(_m_CurrTime),
m_Docs(_m_Docs),
m_OldDocs(_m_OldDocs),
m_OldestProfUrl(InitStr) {};
};

Default assignment/copy-construction/destruction should work if your
template class' overrides of these work.

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

Exact version?

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.

I'd try ruling out bugs (aggregate initializer or getting confused by
non-POD-struct) first, especially since your target RH versions are
archaic. Rewrite Prof as a proper class (and use constructor rather
than aggregate initialization) to bypass these.

Pete Becker wrote:

On 2008-08-28 08:47:33 -0400, anon <anon@no.nosaid:
>

>
Huh? Of course we were talking about C++ programs. The standard C
headers, slightly modified, are standard headers in C++, too.
>

That was my objection: do not use C headers in C++ programs.

On Aug 28, 10:23*am, anon <a...@no.nowrote:

That was my objection: do not use C headers in C++ programs.

You say that probably because you think e.g. <stdio.his not a C++
header. That is wrong. Please accept that e.g. <stdio.his a standard
C++ header.

Ali

On 2008-08-28 13:23:22 -0400, anon <anon@no.nosaid:

Pete Becker wrote:

>
That was my objection: do not use C headers in C++ programs.

Sigh. They're C++ headers, too. That's what the standard says. There's
no good reason not to use them.

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

anon wrote:

DaveJ wrote:

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

Why?




Brian

On 28 août, 20:14, Pete Becker <p...@versatilecoding.comwrote:

[...]

>
Sigh. They're C++ headers, too. That's what the standard says. There's
no good reason not to use them.
>

No.

<stdio.his a C header.
<cstdiois a C++ header.

The use of C headers in a C++ program is possible but deprecated.
It should only be used when the program needs to be compatible with C.

Alexandre Courpron.

On 2008-08-28 14:46:15 -0400, courpron@gmail.com said:

On 28 août, 20:14, Pete Becker <p...@versatilecoding.comwrote:
>
[...]
>

>
No.
>
<stdio.his a C header.
<cstdiois a C++ header.

No, <stdio.his a C header and a C++ header. Look at the standard.

>
The use of C headers in a C++ program is possible but deprecated.

Yes, there was a great deal of wishful thinking in the original
standard, including that. Although they're deprecated, they're not
going to go away. They work just fine.

It should only be used when the program needs to be compatible with C.

I see. You're applying the well-known design principle: never give a
logical reason when you can recite a slogan.

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

On 28 août, 20:58, Pete Becker <p...@versatilecoding.comwrote:

On 2008-08-28 14:46:15 -0400, courp...@gmail.com said:
>

>

>

>

>

>

>
No, <stdio.his a C header and a C++ header. Look at the standard.
>

No, and I'll assume that this is just wrong unless you tell me where
the standard says this explicitly.

There are 3 kinds of headers that can be used in a C++ program :
- C++ headers
- C++ headers for C functionnalities
- C headers

and each of these categories are distinct.

>
Yes, there was a great deal of wishful thinking in the original
standard, including that. Although they're deprecated, they're not
going to go away. They work just fine.
>

This is not a long-term "future proof" statement.

>
I see. You're applying the well-known design principle: never give a
logical reason when you can recite a slogan.

This is not a slogan, this is recommandation ( also made by the C++
standard (17.4.1.2 : footnote 160) ).

C headers should not be used by a C++ program because they are
deprecated. But, anyway, this is also bad practice, because using a C
header should denote (to every programmer reading the program) that
the program must be compatible with C.

Alexandre Courpron.

On 2008-08-28 15:40:54 -0400, courpron@gmail.com said:

On 28 août, 20:58, Pete Becker <p...@versatilecoding.comwrote:

>
No, and I'll assume that this is just wrong unless you tell me where
the standard says this explicitly.

Assume whatever you like.

>
There are 3 kinds of headers that can be used in a C++ program :
- C++ headers
- C++ headers for C functionnalities
- C headers
>
and each of these categories are distinct.

Ah, you're playing word games. Sorry I missed that earlier.

>
>

>
>
This is not a long-term "future proof" statement.

Nor is any other statement about programming, I suppose. Nevertheless,
the C headers are not going to be removed from the C++ standard.

>
>

>
>
This is not a slogan, this is recommandation ( also made by the C++
standard (17.4.1.2 : footnote 160) ).
>
C headers should not be used by a C++ program because they are
deprecated.

As I said, thre's a great deal of wishful thinking in the original
standard. They're not going away.

But, anyway, this is also bad practice, because using a C
header should denote (to every programmer reading the program) that
the program must be compatible with C.
>

Nice circular argument.

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

In article
<12d58e70-0bcf-4ef4-b4e1-34e52db9c43c@26g2000hsk.googlegroups.com>,
courpron@gmail.com wrote:

On 28 ao=FBt, 20:14, Pete Becker <p...@versatilecoding.comwrote:
>
[...]
>

>
No.
>
<stdio.his a C header.
<cstdiois a C++ header.

Everything an ISO C++ program has access to is in the ISO C++ standard;
nothing is automatically included unless the standard includes it,
therefore everything accessible is part of ISO C++. An ISO C++ program can
use <stdio.h>, therefore it's part of ISO C++.

Here's the relevant section from the standard. Note how it states that the
C++ standard library provides these headers, and that while deprecated,
they are part of the current standard, therefore part of the language:

Annex D
Compatibility features
>
1 This clause describes features of the C + + Standard that are
specified for compatibility with existing implementations.
>
2 These are deprecated features, where deprecated is defined as:
Normative for the current edition of the Standard, but not
guaranteed to be part of the Standard in future revisions.

[...]

D.5 Standard C library headers
>
1 For compatibility with the Standard C library, the C++ Standard
library provides the 18 C headers, as shown in Table 100:
>
Table 100‹C Headers
>
<assert.h<iso646.h<setjmp.h<stdio.h<wchar.h>
<ctype.h<limits.h<signal.h<stdlib.h<wctype.h>
<errno.h<locale.h<stdarg.h<string.h>
<float.h<math.h<stddef.h<time.h>
>
2 Every C header, each of which has a name of the form name.h,
behaves as if each name placed in the Standard library namespace by
the corresponding cname header is also placed within the namespace
scope of the namespace std and is followed by an explicit
using-declaration (7.3.3).