acnutch
acnutch

Reputation: 104

C freeing array and elements fails with double free

I'm new to C and trying to work with the regex library. So far Ive successfully built an array of regex matches (array of strings) and I'm trying to free the memory used when doing so. Code is here:

    #include "basic_curl.h"

//returns an array of strings
//the free_regex_memory should be called when regex results are no longer
//needed
char **regexme(char *_string, const char *reg_to_match, int reg_limit) {

    regex_t preg;
    size_t nmatch = 1;
    regmatch_t pmatch[1];
    int comp_ret;
    int match;
    int start;
    int end = 0;
    int match_len;
    int i;
    int string_offset = 0;

    char **matches = (char **) malloc(sizeof(char *) * reg_limit);

    for (i=0; i < reg_limit; i++) {

    comp_ret = regcomp(&preg, reg_to_match, REG_ICASE|REG_EXTENDED);
    match = regexec(&preg, &_string[string_offset], nmatch, pmatch, 0);

    if (match == 1) {
        puts("No more matches found, rest of the loop will be filled with NULLs");
        break;
    }

    else if (match == 0 ) {
        start = pmatch[0].rm_so;
        end = pmatch[0].rm_eo;

        string_offset += end;
        match_len = end - start;
        printf("%.*s\n", match_len, &_string[string_offset - match_len]);

        //use malloc to find the length and use that instead of limiting array initially
        //http://stackoverflow.com/questions/33003196/cant-copy-string-to-an-array-of-strings-in-c
        matches[i] = malloc(sizeof(char) * (match_len + 1));
        sprintf(matches[i], "%.*s" , match_len, &_string[string_offset - match_len]);

    }
    }

    return matches;
}

int free_regex_memory(char **matches_array) {

    int i = 0;
    while (matches_array[i] != NULL) {
        free(&matches_array[i]);
    }

    //why can't I do this after the above?
    //I get a crash from the below line trying to free the array itself:
    /*
       *** Error in `/home/punk/ClionProjects/curl-ex/src/regmatch': double free or corruption (fasttop): 0x0000000000603010 ***

      Program received signal SIGABRT, Aborted.
      0x00007ffff7a4af79 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
    */
    free(matches_array);

    return 0;

}

int main() {

    char **matches;
    int i =0;

    matches = regexme("0fff 1fc<a href=\"https://www.blahblahblah.com/whatever.php?xxx=r\" keaw 2eafa", 
              "(http|https)://[%/0-9a-zA-Z\\.\?=&#@:]*",
              10);

    //puts(matches[1]);

    while (matches[i] != NULL) {
        puts(matches[i]);
        i++;
    }

    free_regex_memory(matches);

    return 0;
}

Basically the above regexme function extracts regexes from a string and stores them into a dynamically allocated array of strings called "matches" and returns this from the function. This works well.

My problem is that I'd now like to free the memory associated with the array of strings, which is where the free_regex_memory() function comes in. I loop through the array, and free the memory associated with each element in the matches array and then I try to free the array itself. I can do one or the other, either free the array or free its elements. However trying to do both (as in the code above) gives me the error "double free or corruption" (as seen in comments in code above).

So what gives? All the other SO questions I see mention needing to free the malloced array AND elements to properly free memory, but I can't seem to do so. What am I missing?

As an aside being new to C am I doing anything amazingly dumb or inefficient in this code?


Edit: here is my new code based on the comments and answers

#include "basic_curl.h"

//returns an array of strings
//the free_regex_memory should be called when regex results are no longer
//needed
char **regexme(char *_string, const char *reg_to_match, int reg_limit) {

    regex_t preg;
    size_t nmatch = 1;
    regmatch_t pmatch[1];
    int comp_ret;
    int match;
    int start;
    int end = 0;
    int match_len;
    int i;
    int string_offset = 0;

    //char **matches = (char **) malloc(sizeof(char *) * reg_limit);

    void **matches = malloc(sizeof(char *) * reg_limit);

    for (i=0; i < reg_limit; i++) {

    comp_ret = regcomp(&preg, reg_to_match, REG_ICASE|REG_EXTENDED);
    match = regexec(&preg, &_string[string_offset], nmatch, pmatch, 0);

    if (match == 1) {
        puts("No more matches found, rest of the loop will be filled with NULLs");
        break;
    }

    else if (match == 0 ) {
        start = pmatch[0].rm_so;
        end = pmatch[0].rm_eo;

        string_offset += end;
        match_len = end - start;
        printf("%.*s\n", match_len, &_string[string_offset - match_len]);

        //use malloc to find the length and use that instead of limiting array initially
        //http://stackoverflow.com/questions/33003196/cant-copy-string-to-an-array-of-strings-in-c
        matches[i] = malloc(sizeof(char) * (match_len + 1));
        sprintf(matches[i], "%.*s" , match_len, &_string[string_offset - match_len]);

    }
    }

    return matches;
}

int free_regex_memory(char **matches_array) {

    int i = 0;
    //fixed so that i'm no longer dereferencing the array element addresses and incrementing the pointer
    while (matches_array[i] != NULL) {
        free(matches_array[i]);
         i++;
    }

    //this works now
    free(matches_array);

    return 0;

}

int main() {

    char **matches;
    int i =0;

    matches = regexme("0fff 1fc<a href=\"https://www.blahblahblah.com/whatever.php?xxx=r\" keaw 2eafa", 
              "(http|https)://[%/0-9a-zA-Z\\.\?=&#@:]*",
              10);

    //puts(matches[1]);

    while (matches[i] != NULL) {
        puts(matches[i]);
        i++;
    }

    free_regex_memory(matches);

    return 0;
}

Oh and here is basic_curl.h in case anyone wants to compile this:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <curl/curl.h>
#include <regex.h>
#include <sys/types.h>

struct MemWriteData {

    size_t size;
    char *memory;
};

static size_t write_callback(char *ptr, size_t size, size_t nmemb, void *userp);
char **regexme(char *_string, const char *reg_to_match, int reg_limit);
int free_regex_memory(char **matches_array);

Upvotes: 2

Views: 957

Answers (2)

user557597
user557597

Reputation:

More advice -

From Wikipedia:

The memory set aside by malloc is not initialized and may contain cruft: the remnants of previously used and discarded data

When using malloc(), clear the memory block after allocating it:

char **matches = (char **) malloc(sizeof(char *) * reg_limit);
memset( (char *)matches, 0, sizeof(char *) * reg_limit );

Don't depend on a hard end to matches_array pointers, use the limit you've allocated:

void free_regex_memory( char **matches_array, int reg_limit )
{
    int i = 0;
    for (i = 0; i < reg_limit; i++)
    {
        if ( matches_array[i] != NULL )
           free( matches_array[i] );   // initial problem
    }
    free(matches_array);
}

Called from Main:

int main()
{
    char **matches;
    int i =0;

    /////////////////

    free_regex_memory( matches, reg_limit );
    return 0;
}

Upvotes: 1

inetknght
inetknght

Reputation: 4439

You have:

int i = 0;
while (matches_array[i] != NULL) {
    free(&matches_array[i]);
}
  • you're freeing the address of matches_array[i]. I don't believe that's intended.
  • you're never incrementing i

You need code equivalent to this (using a for loop is an option):

int i = 0;
while (matches_array[i] != NULL) {
    free(matches_array[i++]);
}

Upvotes: 2

Related Questions