Memory leak or not?

K

kas

Hi Guys,
Do u see any memory leak issue with following code:-

bool myhook(
mem_if *pThis, .
char *Description
)
{
EXTMEMrange_t temp;

temp.name = new char[strlen(Description)+1]; <-allocation
strcpy(temp.name,Description);
....
....
bool result = AddRange(m_hkdMEMranges,temp);

return result;
}



bool AddRange(std::vector<EXTMEMrange_t> & rangeVector, EXTMEMrange_t
range)
{
....
delete [] range.name; <-deallocation
return true;
}

The temp is passed by value to function Addrange. temp.name is
allocated with new and deleted in Addrange.

Will shallow or deep copy can cause any issue here?

-KAS
 
R

Ron

Hi Guys,
Do u see any memory leak issue with following code:-

bool myhook(
        mem_if          *pThis,            ..
        char                    *Description
        )
{
        EXTMEMrange_t temp;

        temp.name      = new char[strlen(Description)+1]; <-allocation
        strcpy(temp.name,Description);
        ....
        ....
        bool result = AddRange(m_hkdMEMranges,temp);

        return result;

}

bool AddRange(std::vector<EXTMEMrange_t> & rangeVector, EXTMEMrange_t
range)
{
                ....
                delete [] range.name; <-deallocation
                return true;

}

The temp is passed by value to function Addrange. temp.name is
allocated with new and deleted in Addrange.

Will shallow or deep copy can cause any issue here?

It's impossible to tell because you don't provide any indication
of what the copy semantics are (like what do the copy constructor
and copy assignment operator due with name?).

What I do is an absolutely horrendously bad design. Allocation
and deallocation are not very symmetrical. Various external
functions are reaching in and grabbing sensitive internal
parts of a class.

Further in the limited example shown, there doesn't seem to
be any excuse for using a raw pointer and poorly engineered
allocation at all. If temp.name is intended to be a string,
use the std::string class. That way most of your heartaches
with regard to memory management are deferred to an already
debugged system interface with well defined copy behavior.
 
K

kas

Hi Guys,
Do u see any memory leak issue with following code:-
bool myhook(
        mem_if          *pThis,            .
        char                    *Description
        )
{
        EXTMEMrange_t temp;
        temp.name      = new char[strlen(Description)+1]; <-allocation
        strcpy(temp.name,Description);
        ....
        ....
        bool result = AddRange(m_hkdMEMranges,temp);
        return result;

bool AddRange(std::vector<EXTMEMrange_t> & rangeVector, EXTMEMrange_t
range)
{
                ....
                delete [] range.name; <-deallocation
                return true;

The temp is passed by value to function Addrange. temp.name is
allocated with new and deleted in Addrange.
Will shallow or deep copy can cause any issue here?

It's impossible to tell because you don't provide any indication
of what the copy semantics are (like what do the copy constructor
and copy assignment operator due with name?).

What I do is an absolutely horrendously bad design.   Allocation
and deallocation are not very symmetrical.  Various external
functions are reaching in and grabbing sensitive internal
parts of a class.

Further in the limited example shown, there doesn't seem to
be any excuse for using a raw pointer and poorly engineered
allocation at all.   If temp.name is intended to be a string,
use the std::string class.   That way most of your heartaches
with regard to memory management are deferred to an already
debugged system interface with well defined copy behavior.- Hide quoted text -

- Show quoted text -

Here is code of EXTMEMrange

// end address is included! [startAddr:endAddr];
struct MEMrange_t
{
uint64 startAddr;
uint64 endAddr;
char* name;
IMemory64* pin;

MEMrange_t() {startAddr = 0; endAddr = 0; name = "NONAME"; pin =
NULL;};
MEMrange_t(char * name_) {startAddr = 0; endAddr = 0; name = name_ ;
pin = NULL;};
MEMrange_t(uint64 startAddr_, uint64 endAddr_, char * name_,
IMemory64* pin_) {startAddr = startAddr_; endAddr = endAddr_; name =
name_ ; pin = pin_;};
};

struct EXTMEMrange_t : public MEMrange_t
{
};

I am intersted if memory leak can happen than why?
 
K

Kai-Uwe Bux

kas said:
Hi Guys,
Do u see any memory leak issue with following code:-

bool myhook(
mem_if *pThis, .
char *Description
)
{
EXTMEMrange_t temp;

temp.name = new char[strlen(Description)+1]; <-allocation
strcpy(temp.name,Description);
....
....
bool result = AddRange(m_hkdMEMranges,temp);

return result;
}



bool AddRange(std::vector<EXTMEMrange_t> & rangeVector, EXTMEMrange_t
range)
{
....
delete [] range.name; <-deallocation
return true;
}

The temp is passed by value to function Addrange. temp.name is
allocated with new and deleted in Addrange.

That is just poor design (it requires too much global knowledge to establish
correctness); but it is not per se a memory leak.

Nonetheless, the code will leak memory if an exception is thrown anywhere in
the "...." parts. To protect against that, you can either use RAII (e.g.,
std::string comes to mind here) or a try-catch block.

Will shallow or deep copy can cause any issue here?

Yes. Suppose that EXTMEMrange_t had deep copy semantics. The deep copy would
be triggered in passing range by value into AddRange(). Then the
deallocation will hit the wrong pointer, i.e., the code supposes that
EXTMEMrange_t does not have deep copy semantics. Also, if EXTMEMrange_t
has deep copy semantics, it would probably deallocate its resources in the
destructor, which would lead to a double deletion in the above snippet.


Recommendation: use std::string.


Best

Kai-Uwe Bux
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads


Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top