views:

148

answers:

7

Quick question: I am a C# guy debugging a C++ app so I am not used to memory management.

In the following code:

for(int i = 0; i < TlmMsgDB.CMTGetTelemMsgDBCount(); i++)
  {
    CMTTelemetryMsgCls* telm = TlmMsgDB.CMTGetTelemetryMsg(i);
    CMT_SINT32_Tdef id = telm->CMTGetPackingMapID();

    ManualScheduleTables.SetManualMsg(i,id);
    ManualScheduleTables.SetManExec(i,false);
  }

Am I leaking memory every iteration b/c of CMTTelemetryMsgCls* telm = TlmMsgDB.CMTGetTelemetryMsg(i);? The "CMTGetTelemetryMsg(int)" method returns a pointer.

Do I have to "delete telm;" at the end of each iteration?

+3  A: 

There is no global answer for this question.

It really depends on how the person implemented the function with the return value.

  • It may return a variable allocated on the heap and expect you to delete it
  • It may return a member variable pointer not even on the heap
  • If its return value is on the heap, TlmMsgDB may do its own memory management

You need to look on the implementation of the function you're calling: TlmMsgDB.CMTGetTelemetryMsg

Brian R. Bondy
... or the documentation.
Juliano
@Juliano: right.
Brian R. Bondy
the library returns the address of the data like so: return Do i have to delete the pointer in this case?
Bobby
@Bobby: That still depends what `TelemetryMsgDB[i]` is. A pointer is simply a variable that holds an address. What's inside of it can be on the heap and may not be. Was it created with new or new[]? Does the class do its own memory management? These are questions I can't answer.
Brian R. Bondy
I don't think you need to delete (nor should delete). The returned pointers are pointers into an array (unless `TelemetryMsgDB` is an instance of a class implementing `operator[]`, but that seems unlikely). Whether the array had automatic storage or was dynamically allocated is not really an issue here, in neither case you should delete pointers into it. (If it was dynamically allocated then the library is probably cleaning up for itself, or else you need to do it somewhere else)
David Rodríguez - dribeas
+2  A: 

it depends. if this method TlmMsgDB.CMTGetTelemetryMsg allocates memory then you have to free it. If it just returns pointer to something existent then don't

Andrey
+1  A: 

If TlmMsgDB.CMTGetTelemetryMsg(i) is allocating and returning a pointer to a new CMTTelemetryMsgCls object, then yes. You should probably delete that object when you're done with it, assuming it's safe to do so. The library you're working with may also perform some kind of garbage collection, so read the documentation.

Andrew Noyes
+1  A: 

Depends. The problem with using pointers like that is unless you read the documentation for the CMTGetTelemetryMsg function you have no idea if it created a new pointer for you and gave you ownership, or if it's just giving you a handle on internal data...Either way it's bad practice. If the docs say that you now own the pointer, then yes you have to delete on every loop. If the pointer is owned and managed by TlmMsgDB then you're fine.

Chris H
+1  A: 

Not necessarily. It depends on the internals of TlmMsgDB.CMTGetTelemetryMsg(i).

If it returns a new object every time ("new Something()"), then yes, it's leaking memory, absent some sort of memory management. However, the fact that it's called "get"-something indicates that it may create the object once, and then return the same pointer when called again. That's probably what it does, I'd guess.

Hope that clears things up a bit. :) These things are a bit strange at first. ;)

Helgi Hrafn Gunnarsson
+1  A: 

Everyone is right, it depends. However, that doesn't really help you figure out whether or not you need to call delete, aside from someone or something telling you what the behavior is. I can think of a couple of ways to figure it out yourself:

1) In your debugger, break before the loop starts, then do a stop in whatever function allocates memory (eg malloc,mmap,brk) and see if it stops. Not fool proof alone, but you can try to step around to see the address returned and then check the address that is ultimately returned by the CMTGetPackingMapID function.

2) A less precise but simpler way to say 'is there allocation happening at all inside this function' is to use your sysems version of strace/ltrace. perhaps sleep right before the loop to make it easy to tell where you are in the trace.

3) Watch your system's equivalent of top to see if you are leeking

Note that C++ is an evil language and nothing can really tell you for sure except the code. For instance, there is nothing that prevents the actually correct behavior from being having to delete some member 10 levels deep inside the object you have a pointer to. Or even god forbid to have to free() it because actually the class calls malloc somewhere deep down in it's depths.

frankc
A: 

Try using the profiler. It can supply you with some sort of useful information about the memory management running inside of the process being profiled. Or, even better, you can use something like SoftICE, it allows you to set breakpoint at any function that allocates memory, for example HeapAllocEx. Using such tool you will definitely determine the real behavior of 3-rt party code, even without sources.

leonard