malloc problem in C++

[Jason]

I've inherited some old C code that I've been tasked to convert to C++.
I've been running into some problems with memory allocation.

First of all, is there any problems using "malloc" in C++? I know using
"new" is preferable, but for now I have to use what is provided.

Second, do FILE pointers need to have memory allocated?

My specific problem is this: (all code examples are abbreviated for
clarity)

I have a class

class TObjectDef
{
public:
char *cpFileName;
int iRecordLength;
int iCurrentRecord;
long lRecordCount;
FILE *tpFilePointer
char cpRecBuf[2048];
}

which is initialized

gsPpk = new TObjectDef();

then, much later in the code (about 80k lines later), a float array is
initialized from this structure

(definition
struct DataSegments
{
char caDataId[6];
int iDataCount;
float *fpArr1;
float *fpArr2;
}
)

like so

void vSetFpArray(struct DataSegments *spDataSeg, float *fpArr1, float
*fpArr2)
{
int iDataCount;
int iDataIndex;

iDataCount = spDataSeg->iDataCount;
spDataSeg->fpArr1 = (float *)malloc(sizeof(float) * iDataCount);
spDataSeg->fpArr1 = (float *)malloc(sizeof(float) * iDataCount);

for (iDataIndex = 0; iDataIndex < iDataCount; iDataIndex++)
{
spDataSeg->fpArr1[iDataIndex] = fpArr1[iDataIndex];
spDataSeg->fpArr1[iDataIndex] = fpArr1[iDataIndex];
}
return;
}

The function is called like

vSetFpArray(&sDataSegments[currInd],floatArray1,floatArray2);

where the variables are defined as
static DataSegments sDataSegments[512];
float floatArray1[1024];
float floatArray2[1024];

The function above gets called just fine 318 times. On the 319th iteration,
when "malloc" is called, it is overwriting part of the previously defined
class (which is global to this function). The tpFilePointer goes from being
NULL to being "1" after the first malloc, and "2" after the second. (as in,
pointing to memory address :00000002).

Can anyone see any obvious problems with all this that I may have
overlooked? (Besides the obvious problems with global classes and such.) I
apologize if this question didn't make any sense, I'm happy to provide
clarity.

- Jason

 

[red floyd]

I've inherited some old C code that I've been tasked to convert to C++.

Is there a reason you can't just use the C code as a library, and just
extern "C" the header?

I've been running into some problems with memory allocation.

First of all, is there any problems using "malloc" in C++? I know using
"new" is preferable, but for now I have to use what is provided.

No. You just need to remember that malloc needs free. You can't pass a
malloc'ed entity to delete.

Also, malloc'ed memory is uninitialized, until you fill it with
something, it's UB to read that memory.

If you're converting the code anyways, why can't you change malloc
to new?

Second, do FILE pointers need to have memory allocated?

No. It's handled by fopen().

 

[Victor Bazarov]

I've inherited some old C code that I've been tasked to convert to
C++. I've been running into some problems with memory allocation.

First of all, is there any problems using "malloc" in C++? I know
using "new" is preferable, but for now I have to use what is provided.

No, there are no problems with 'malloc', except that it does not
initialise the memory it allocates.

Second, do FILE pointers need to have memory allocated?

FILE pointers are those you obtain from the system by means of the
'fopen' function calls. You need not allocate anything.

My specific problem is this: (all code examples are abbreviated for
clarity)

I have a class

class TObjectDef
{
public:
char *cpFileName;
int iRecordLength;
int iCurrentRecord;
long lRecordCount;
FILE *tpFilePointer
char cpRecBuf[2048];
}

which is initialized

gsPpk = new TObjectDef();

Assuming that 'gsPpk' is declared as a pointer to TObjectDef.

then, much later in the code (about 80k lines later), a float array is
initialized from this structure

(definition
struct DataSegments
{
char caDataId[6];
int iDataCount;
float *fpArr1;
float *fpArr2;
}
)

like so

void vSetFpArray(struct DataSegments *spDataSeg, float *fpArr1, float
*fpArr2)
{
int iDataCount;
int iDataIndex;

iDataCount = spDataSeg->iDataCount;
spDataSeg->fpArr1 = (float *)malloc(sizeof(float) * iDataCount);
spDataSeg->fpArr1 = (float *)malloc(sizeof(float) * iDataCount);

I am sure you meant

spDataSeg->fpArr2 =

here, didn't you? Otherwise you assign to the same variable twice,
which means you leak memory and leave 'fpArr2' unchanged.

for (iDataIndex = 0; iDataIndex < iDataCount; iDataIndex++)
{
spDataSeg->fpArr1[iDataIndex] = fpArr1[iDataIndex];
spDataSeg->fpArr1[iDataIndex] = fpArr1[iDataIndex];

Again, those two statements are _exactly_the_same_. Did you mean
to do

spDataSeg->fpArr2[iDataIndex] = fpArr2[iDataIndex];

on the second line?

}
return;
}

The function is called like

vSetFpArray(&sDataSegments[currInd],floatArray1,floatArray2);

where the variables are defined as
static DataSegments sDataSegments[512];
float floatArray1[1024];
float floatArray2[1024];

The function above gets called just fine 318 times. On the 319th
iteration, when "malloc" is called, it is overwriting part of the
previously defined class (which is global to this function). The
tpFilePointer goes from being NULL to being "1" after the first
malloc, and "2" after the second. (as in, pointing to memory address
:00000002).

Can anyone see any obvious problems with all this that I may have
overlooked? (Besides the obvious problems with global classes and
such.) I apologize if this question didn't make any sense, I'm happy
to provide clarity.

Yes, the obvious problems I've pointed out. The non-obvious are
most likely beyond the posted code.

V

 

[Jason]

[...]
First of all, is there any problems using "malloc" in C++? I know
using "new" is preferable, but for now I have to use what is provided.

No, there are no problems with 'malloc', except that it does not
initialise the memory it allocates.

But it _does_ allocate memory, as in it locks memory on the heap for that
specific pointer?

FILE pointers are those you obtain from the system by means of the
'fopen' function calls. You need not allocate anything.

Thanks, that's what I suspected.

[...]
gsPpk = new TObjectDef();

Assuming that 'gsPpk' is declared as a pointer to TObjectDef.
Correct.

[...]
spDataSeg->fpArr1 = (float *)malloc(sizeof(float) * iDataCount);
spDataSeg->fpArr1 = (float *)malloc(sizeof(float) * iDataCount);

I am sure you meant

spDataSeg->fpArr2 =

here, didn't you? Otherwise you assign to the same variable twice,
which means you leak memory and leave 'fpArr2' unchanged.

Yes, you are correct. I changed the variable names for this example, and
copy/pasted it into the code. Missed updating the second reference, I
apologize for the confusion. The old variable names are part of a
proprietary application, needed to change them for my own job security.

Again, those two statements are _exactly_the_same_. Did you mean
to do

spDataSeg->fpArr2[iDataIndex] = fpArr2[iDataIndex];

on the second line?

Again, I apoligize for the confusion.

<shrug> Impossible to tell what that's due.

Any suggestions on things I could be looking for while debugging? There are
no errors until it gets further down in the code, and an attempt is made to
initialize and use that file pointer.

Yes, the obvious problems I've pointed out. The non-obvious are
most likely beyond the posted code.

Besides my typos, everything looks fine? Thanks for your advice, that's
what I needed to know. I am still relatively new to the C++ environment,
being an old Delphi programmer, so outside perspective is always
appreciated.

- Jason

 

[Jason]

Is there a reason you can't just use the C code as a library, and just
extern "C" the header?

The original code is ~2M lines, broken into 18 separate applications that
call each other with system() commands, currently developed and running on a
UNIX terminal. I'm not really sure of the extern "C" functionality, but
wouldn't the UNIX code have trouble compiling under my system? It uses
things like "termcap" that are unique to the unix environment.

No. You just need to remember that malloc needs free. You can't pass a
malloc'ed entity to delete.

Thanks for the tip.

Also, malloc'ed memory is uninitialized, until you fill it with
something, it's UB to read that memory.
UB?

If you're converting the code anyways, why can't you change malloc
to new?

I can, and I will, but because of deadlines and the expansiveness of this
project, currently doing so is simply too time prohibitive.

No. It's handled by fopen().

Thanks, I assumed as much, but you can never be too sure.

- Jason

 

[Victor Bazarov]

[...]
First of all, is there any problems using "malloc" in C++? I know
using "new" is preferable, but for now I have to use what is
provided.

No, there are no problems with 'malloc', except that it does not
initialise the memory it allocates.

But it _does_ allocate memory, as in it locks memory on the heap for
that specific pointer?

Sure. If you look at some popular implementations of 'new' and
'new[]', you'll see that internally they use 'malloc', actually.

[..]

<shrug> Impossible to tell what that's due.

Any suggestions on things I could be looking for while debugging?

If 'malloc' somehow overrides (steps onto) some memore you're still
using for some other object, it could be only two things: the heap
is corrupt (your program writes beyond the bounds of a dynamically
allocated object, thus crossing over to memory that doesn't really
belong to it, OR your "part of previously defined class" was somehow
transferred from under your control and it's now part of the heap
that 'malloc' is free to reuse.

What you should do is to get the tool that would allow you to debug
memory allocations and access. Rational Purify, BoundsChecker, come
to mind.

There are no errors until it gets further down in the code, and an
attempt is made to initialize and use that file pointer.

Memory access troubles are the worst, and if you get them under your
control, you're going to be very happy.

Besides my typos, everything looks fine? Thanks for your advice,
that's what I needed to know. I am still relatively new to the C++
environment, being an old Delphi programmer, so outside perspective
is always appreciated.

Here is something you might want to consider: do NOT use dynamic
memory allocated directly by you, instead use standard containers.
Switch to using 'std::vector<float>' instead of 'malloc'ed 'float*'.
Easier to debug, for sure.

V

 

[Victor Bazarov]

UB?

"Undefined behaviour"

V

 

[Jason]

"Undefined behaviour"

Well that makes a lot more sense than "Undistributed Budget".

*sigh*

- Jason

 

[Default User]

I've inherited some old C code that I've been tasked to convert to
C++. I've been running into some problems with memory allocation.

First of all, is there any problems using "malloc" in C++? I know
using "new" is preferable, but for now I have to use what is provided.

As the others have mentioned, malloc() will work for allocating raw
buffers. One difference between C and C++ is that C has automatic
conversion of void pointers to object pointers, while C++ does not.
It's possible that the old code could be written like:

int *p = malloc(8 * sizeof(int));

That won't work in C++. That being said, it's extremely common for C
code to cast the pointer from malloc() anyway, even though it's not
needed and not really a good idea in C, as it can hide a nasty error.



Brian

 

[Jason]

But it _does_ allocate memory, as in it locks memory on the heap for
that specific pointer?

Sure. If you look at some popular implementations of 'new' and
'new[]', you'll see that internally they use 'malloc', actually.

Well that explains why I'm still getting the same error using "new" and
"delete".

If 'malloc' somehow overrides (steps onto) some memore you're still
using for some other object, it could be only two things: the heap
is corrupt (your program writes beyond the bounds of a dynamically
allocated object, thus crossing over to memory that doesn't really
belong to it, OR your "part of previously defined class" was somehow
transferred from under your control and it's now part of the heap
that 'malloc' is free to reuse.

The old code is horrendous, with objects being used before they exist, and
other inappropriate actions. I'm really not surprised that this is
happening, just confused about how to solve it with no *apparent* errors. I
was hoping that this was going to be more of an oversight on my part than
anything. The object that is being overwritten is most likely being
re-initialized somewhere. If I re-allocate an object doing:

gsPpk = new ObjectDef();

then later in the code do

gsPpk = new ObjectDef();

again, what happens? Is the original instance of the object lost and that
memory locked?

What you should do is to get the tool that would allow you to debug
memory allocations and access. Rational Purify, BoundsChecker, come
to mind.

Thanks, I will definitely check into some options.

Memory access troubles are the worst, and if you get them under your
control, you're going to be very happy.

c:\stupidproject>del /f /s *.*

would make me happy.

Here is something you might want to consider: do NOT use dynamic
memory allocated directly by you, instead use standard containers.
Switch to using 'std::vector<float>' instead of 'malloc'ed 'float*'.
Easier to debug, for sure.

If only it were that simple. If I could merely do a gloabl search/replace,
my life would be so much simpler.

- Jason

 

[Jason]

As the others have mentioned, malloc() will work for allocating raw
buffers. One difference between C and C++ is that C has automatic
conversion of void pointers to object pointers, while C++ does not.
It's possible that the old code could be written like:

int *p = malloc(8 * sizeof(int));

That won't work in C++. That being said, it's extremely common for C
code to cast the pointer from malloc() anyway, even though it's not
needed and not really a good idea in C, as it can hide a nasty error.

There were a few places in the old code like this, but things like that
throw compilation errors. The code now compiles fine, but I'm getting
memory problems at run-time.

- Jason

 

[Victor Bazarov]

But it _does_ allocate memory, as in it locks memory on the heap for
that specific pointer?

Sure. If you look at some popular implementations of 'new' and
'new[]', you'll see that internally they use 'malloc', actually.

Well that explains why I'm still getting the same error using "new"
and "delete".

Make sure you do the right 'delete' -- "delete[]" for arrays.

The old code is horrendous, with objects being used before they
exist, and other inappropriate actions. I'm really not surprised
that this is happening, just confused about how to solve it with no
*apparent* errors. I was hoping that this was going to be more of an
oversight on my part than anything. The object that is being
overwritten is most likely being re-initialized somewhere. If I
re-allocate an object doing:
gsPpk = new ObjectDef();

then later in the code do

gsPpk = new ObjectDef();

again, what happens?

Nothing awful, most likely.

Is the original instance of the object lost and
that memory locked?

The original instance is "lost" in terms of gaining access to it, but
the memory is still allocated. This is what's known to be "a memory
leak", since the memory is (and cannot be) deallocated because the
pointer to that memory has been lost when you override the value in
the 'gsPpk'. Unless, of course, somebody holds onto the value of the
pointer, which in itself may be a BAD IDEA(tm).

[..]

Memory access troubles are the worst, and if you get them under your
control, you're going to be very happy.

c:\stupidproject>del /f /s *.*

would make me happy.

I hear you.

If only it were that simple. If I could merely do a gloabl
search/replace, my life would be so much simpler.

Refactoring old C code is not much fun. But it's often what you have
to do and it's better done right. That includes stopping the use of
"naked" pointers for arrays and switching to using proper standard
containers. While it looks daunting at first, you will save more time
later if you have done that.

Talk you your boss, ask for an extension of couple of days, back your
project up in a reliable location, then comb those "naked" pointers
out and replace them with vectors. You'll be glad you do.

V

 

[Ian Collins]

There were a few places in the old code like this, but things like that
throw compilation errors. The code now compiles fine, but I'm getting
memory problems at run-time.

Use whatever memory access tools your platform has to help track these down.

 

[Jason]

I've inherited some old C code that I've been tasked to convert to C++.
I've been running into some problems with memory allocation.

[snip snip]

In case anyone was wondering how this turned out, it actually turned out to
be all happening in a totally different part of the code. I also found out
that initializing an object twice causes very undesirable results.

gsFoo = new FooObject;

.....

gsFoo = new FooObject;

It seems that my code was still trying to use the old instance of the object
after this point, which is obviously not a good thing.

Anyways, thanks for all your help.. it's getting there.

- Jason