1

I am having an issue with memory leaks while using realloc inside a function that I want to keep as void. The code I am building is for a sparse matrix data type conversion from "CSR" to a delta units format called "CSR-DU".

The header file for the offending function containing realloc:

void csr_to_csrdu(..., int *ctl_size, uint64_t **ctl, ...);

ctl is a double pointer pointing to the array data that will be modified and reshaped when the array fills up via realloc. The array is dynamically sized and there is no way to guess the final size prior to execution. Hence the need for a pointer for the array size and a double pointer to hold onto the memory address of the data array back in main().

In main():

ctl = (uint64_t **) malloc( sizeof(uint64_t *) );
ctl_data = (uint64_t *) malloc( *ctl_size * sizeof(uint64_t) );

*ctl= ctl_data; // point to memory data

// call data type conversion
csr_to_csrdu(..., ctl_size, ctl, ...);

// compute sparse matrix-vector multiplication w/ ctl
spmv_csrdu(..., ctl_size, *ctl, ...);

// free ctl data - my problem!
free( ??? );

Inside the function, the realloc looks something like this (some psuedo code):

if( ctl_index >= *ctl_size )
{
    int estimate  = get_estimate();
    tempPtr = realloc(*ctl, (*ctl_size + estimate)*sizeof(uint64_t) );

    if( tempPtr  == NULL ){ 
      print_fatal_error();
      exit();
    }else{ 
      *ctl = tempPtr;
    }
}

However, I can't seem to figure out how to free the "ctl_data" after a realloc has occurred inside the function. The address that I originally had in main() has been destroyed. Things I have tried to no avail:

// both cause "double free or corruption" crash
free( ctl_data );   
free( *ctl );

I am not sure how to proceed here. Is there any way to make it so that I can safely free the "tempPtr" that was created inside the function?

5
  • In C you do not cast malloc - please look this up Commented Jan 17, 2019 at 22:51
  • What you are showing is something of a code puzzle. Please consolidate to make a minimal reproducible example. Commented Jan 17, 2019 at 22:53
  • ctl_data continues to exist and the new ctl[0] points at it. Commented Jan 17, 2019 at 22:53
  • 1
    I'd say, don't free both *ctl and ctl_data, but do free both *ctl and ctl (in that order). Commented Jan 17, 2019 at 23:09
  • It's heartwarming to see code that saves the result of realloc into a different variable than the one that is passed in holding the original memory address. Commented Jan 18, 2019 at 0:53

1 Answer 1

2

Short answer:

free(*ctl);
free(ctl);

When you call realloc, it may or may not free the pointer you originally allocated (what was ctl_data in main), which means ctl_data may no longer be valid to free, and thus shouldn't be (and you should not try to access whatever memory it originally pointed to). In fact, it is fairly redundant to begin with, at least as far as the visible code is concerned. Lacking some other use you're not showing, it'd be cleaner to just have:

ctl = malloc( sizeof(uint64_t *) );
*ctl = malloc( *ctl_size * sizeof(uint64_t) );    

Going one step further, it might be even cleaner just to use a single pointer, and a single allocation:

uint64_t *ctl_data = malloc(*ctl_size * sizeof(uint64_t));
...
csr_to_csrdu(..., ctl_size, &ctl_data, ...);
...
spmv_csrdu(..., ctl_size, ctl_data, ...);
...
free(ctl_data);
Sign up to request clarification or add additional context in comments.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.