Unable to spot Memory leak issue in below code

102 Views Asked by At

I am very new to C++. I am facing memory leak issue in my c++ code. Please see the below mentioned piece of code, which is causing the issue.

void a()
{
    char buffer[10000];
    char error_msg[10000];
    char log_file[FILENAME_L] = "error_log.xml";

    FILE *f;
    f = fopen(log_file,"r");
    while (fgets(buffer, 1000, f) != NULL)
    {
        if (strstr(buffer, " Description: ") != NULL)
        {
            strcpy(error_msg, buffer);
        }
    }
    fclose(f);  
    actual_error_msg = trimwhitespace(error_msg);
}

Can anyone please suggest on this. Do I need to use malloc instead of hardcoded size of array?

2

There are 2 best solutions below

4
On BEST ANSWER

It seems that there is undefined behaviour if variable actual_error_msg is a global variable and function trimwhitespace does not dynamically alocate memory for a copy of error_msg

actual_error_msg = trimwhitespace(error_msg);

So when the function finishes its execution pointer actual_error_msg will be invalid.

Can anyone please suggest on this

I am suggesting to allocate dynamically memory for a copy of error_msg within function trimwhitespace. Or if you already do it yourself then check whether the memory is freed in time.:)

Take into account that it looks strange that buffer is declared with the size equal to 10000 while in the fgets there is used magic number 1000.

char buffer[10000];
//,,,
while (fgets(buffer, 1000, f) != NULL)
0
On

TL;DR - In the code snippet shown above, there is no memory leak.

Do I need to use malloc instead of hardcoded size of array?

I think, you got confused by the possible underuse of char buffer[10000]; and char error_msg[10000];. These arrays are not allocated dynamically. Even the arrays are not used to their fullest capacities, there is no memory leak here.

Also, as Mr. @Vlad metioned rightly about another much possible issue in your case, actual_error_msg being a global, if the trimwhitespace() function does not have a return value which is having a global scope, (i.e., stays valid after the a() has finished execution), it may possibly lead to undefined behaviour.

To avoid that, make sure, trimwhitespace() function is either returning (assuming return type is char *)

  • a pointer with dynamic memory allocation (Preferred)
  • base address of a static array. (bad practice, but will work)

To elaborate, from the Wikipedia article about "memory leak"

In computer science, a "memory leak" is a type of resource leak that occurs when a computer program incorrectly manages memory allocations in such a way that memory which is no longer needed is not released. ...

and

.. Typically, a memory leak occurs because dynamically allocated memory has become unreachable. ...

When memory is being allocated by your compiler, there is no scope of memory leak, as the memory (de)allocation is managed by the compiler.

OTOH, with dynamic memory allocation, allocation of memory is performed at runtime. Compiler has no information about the allocation, memory is allocated programatically, hence need to be freed programatically, also. Failing to do so leads to the "memory leak".