Skip to content

Commit

Permalink
Memory leak fixed
Browse files Browse the repository at this point in the history
  • Loading branch information
@minyoung90
minyoung90 committed Jun 13, 2019
1 parent 695dc53 commit 5c19ba4
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions development/src/GXBytebuffer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,11 @@ void CGXByteBuffer::Set(const void* pSource, unsigned long count)
{
m_Capacity += count + VECTOR_CAPACITY;
}

if (m_Data != NULL)
{
free(m_Data);
}
m_Data = (unsigned char*)realloc(m_Data, m_Capacity);
}
memcpy(m_Data + m_Size, pSource, count);
Expand Down

3 comments on commit 5c19ba4

@gurux01
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

Your fix will delete old data. There is no need to free because realloc will take care of that.

@minyoung90
Copy link
Author

@minyoung90 minyoung90 commented on 5c19ba4 Jun 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,
I think it is still dangerous because 'm_Data = (unsigned char*)realloc(m_Data, m_Capacity);' uses 'm_Data ' as function parameter & it's result. so, when it fails, we will lose the memory of original 'm_Data'.

I hope this will help you.
https://stackoverflow.com/questions/9071566/is-it-safe-to-use-realloc

Sorry for no background DLMS/COSEM and this source ( I am currently learning it ) but, if preserving old data is important, I suggest that keep 'm_Data' until 'realloc()' is resulted safely and then copy the result into 'm_Data'.

@gurux01
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

We'll change this to work in the same way as ANSI C version.

            unsigned char* tmp = (unsigned char*)realloc(arr->data, capacity);
            //If not enought memory available.
            if (tmp == NULL)
            {
                return DLMS_ERROR_CODE_OUTOFMEMORY;
            }
            arr->data = tmp;

Please sign in to comment.