Freeing memory in recursive function: free(): double free detected in tcache2 (beginner question)

Originally Posted by lxs602 I hope this isn't a stupid question.

No,it isn't.

Originally Posted by lxs602 I know this method is not the best way, but I tried to correct just as an exercise. How can I make ptr NULL to escape the while loop above?

First of all you either use it as a linked list or an array. Do not mix them, that is just confusing.
So as an array:

for( int a = 0; N > a; ++a) {
    free table[a];
    table[a] = NULL;
}

And as a linked list:

ptr = table[0];
         
while (ptr != NULL)
{         
    p2 = ptr;
    ptr = ptr->next;
    free(p2);
}

But in this case you need to manage that array additionally

@lxs602
Following code is a sample code.
Few years ago I have seen following kind of code(ptr/p2/any pointer) being used by different process(using fork)

for( int a = 0; a < N; ++a) {
    if( NULL != table[a] )
    {
        free table[a];
        table[a] = NULL;
    }
}
// ...
ptr=table[0];
while( NULL != ptr )
{
    p2 = ptr;
    ptr = ptr->next;
    if( NULL != p2 ) // Hence it is better to validate NULL exception since this code in future may be changed to be used by multiple threads/process
    {
        free(p2);
        p2 = NULL;
    }
}

01)
Always use:
a <= N instead of N <= a
I have seen during 2003 to 2008 to view new/oldest developers could have missed < operator and that would report a bug report.
02)
same thing applicable at while( NULL != ptr )
03)
Better to use:
if( NULL != p2 )
{
free(p2);
p2 = NULL;
}
since I have seen free exceptions when my program was using external library using my pointer(rfcsdk and nwrfcsdk) at Linux/HP-UX/SunOS/AIX/CYGWIN_NT at Windows

Originally Posted by murugesandins 03) Better to use: if( NULL != p2 ) { free(p2); p2 = NULL; } since I have seen free exceptions when my program was using external library using my pointer(rfcsdk and nwrfcsdk) at Linux/HP-UX/SunOS/AIX/CYGWIN_NT at Windows

Even if you were using some deallocation function which didn't check for NULL, the check here is doubly redundant because it's already checked in the loop condition, and the ptr->next expression would have caused a segfault if it was NULL.

Originally Posted by ntubski redundant because it's already checked in the loop condition,

I don't have the full code which I have seen during 2008 to 2019.
The posted code was a sample code.
I have written the code based on the analysis I have seen to prevent core dump issue based on external library and based on the compiler (g++.exe/gcc.exe/aCC/xlC/CC/cc) at different OS(CYGWIN_NT/HP-UX/SunOS/AIX/*NIX*)
My comment was:
Handle free using related if condition when it is required always since it may create core dump issue at particular OS alone due to usage of external libraries.
external libraries used to inform the error was not from their side which I have seen during 2008 to 2013 => They have closed my bug report only one time out of 13 bugs I reported.

Originally Posted by murugesandins I don't have the full code which I have seen during 2008 to 2019. The posted code was a sample code. ... external libraries used to inform the error was not from their side which I have seen during 2008 to 2013 => They have closed my bug report only one time out of 13 bugs I reported.

Code you saw in previous years but cannot produce here is irrelevant to this thread, as is discussion of unnamed external libraries. Please confine discussion to the questions asked and relevant examples.

And for benefit of those following the discussion, within the context of this thread the point made by ntubski is actually correct based only on the code posted (i.e. without reference to other sources): The NULL test is twice redundant and unnecessary...

Originally Posted by murugesandins @lxs602 ... while( NULL != ptr ) { p2 = ptr; ptr = ptr->next; if( NULL != p2 ) <- Unnecessary { free(p2); p2 = NULL; } }

Very often, code is simply written as while(ptr) ... since NULL corresponds to zero = false.

However – I have no personal objections to clarity in code: if you want to add that "if p2 is not null," then by all means feel free.

FYI, usually I would write code like this ... like this:

while (ptr) {    // or: while (ptr != NULL) ...
  p2 = ptr->next;
  free(ptr);
  ptr = p2;
}

If you are inside the "while" loop, you know that ptr isn't NULL because you are here.

But again, clarity: "it works," "you can prove it (by testing)," and "it is clear to the human reader." The compiler doesn't really care – it knows how to "optimize away" things anyway.

I explicitly set freed pointers to NULL, as I've said, specifically so that any other code which encounters this pointer won't do the wrong thing. And, now I don't have to care where it might be. In any case, I don't have to "test for NULL" because free() recognizes and ignores NULL.