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

Jason wrote:No, there are no problems with 'malloc', except that it does not
initialise the memory it allocates.
FILE pointers are those you obtain from the system by means of the
'fopen' function calls. You need not allocate anything.
Assuming that 'gsPpk' is declared as a pointer to TObjectDef.
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.
Again, those two statements are _exactly_the_same_. Did you mean
to do

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

on the second line?
<shrug Impossible to tell what that's due.
Yes, the obvious problems I've pointed out. The non-obvious are
most likely beyond the posted code.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

"Victor Bazarov" <v.Abazarov@comAcast.netwrote in message
news:fan3ip$713$1@news.datemas.de...But it _does_ allocate memory, as in it locks memory on the heap for that
specific pointer?
Thanks, that's what I suspected.
Correct.
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, I apoligize for the confusion.
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.
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 wrote:Sure. If you look at some popular implementations of 'new' and
'new[]', you'll see that internally they use 'malloc', actually.
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.
Memory access troubles are the worst, and if you get them under your
control, you're going to be very 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.

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

Jason wrote:"Undefined behaviour"

V
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

"Victor Bazarov" <v.Abazarov@comAcast.netwrote in message
news:fan5us$ej4$1@news.datemas.de...Well that makes a lot more sense than "Undistributed Budget".

*sigh*

- Jason

Jason wrote:
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

"Victor Bazarov" <v.Abazarov@comAcast.netwrote in message
news:fan5sq$eba$1@news.datemas.de...Well that explains why I'm still getting the same error using "new" and
"delete".
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?
Thanks, I will definitely check into some options.
c:\stupidproject>del /f /s *.*

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

- Jason

"Default User" <defaultuserbr@yahoo.comwrote in message
news:5j8l0tF2h69d8U1@mid.individual.net...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

Jason wrote:Make sure you do the right 'delete' -- "delete[]" for arrays.
Nothing awful, most likely.
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).
I hear you.
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
--
Please remove capital 'A's when replying by e-mail
I do not respond to top-posted replies, please don't ask

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

--
Ian Collins.

"Jason" <us.NOSPAM@jswynn.NOSPAM.comwrote in message
news:46cf1081$0$14043$baae4c71@news.mindlink.net.. .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