C Invalid pointer in free() call

I am writing a function to find variables in a string using regular expression. That functionality works fine, but when I try to free the temporary char* that holds the string开发者_如何转开发 I'm evaluating, glibc invokes with an invalid pointer error and the program aborts. In the code below, if the while loop is never entered, no crash occurs.

What am I doing incorrectly?

int parse_variables(size_t read_len)
{
    regex_t comp_regex;
    int start = 0;
    char *command_copy = malloc(sizeof(command));
    strcpy(command_copy, command);
    if (regcomp(&comp_regex, "[$][^0-9_][A-Z0-9_]+", REG_EXTENDED) != 0)
    {
        pmesg(1, "Regex compilation failed. Not parsing for variables.\n");
        return -1;
    }
    regmatch_t pmatch;
    int var_match = regexec(&comp_regex, command_copy+start, comp_regex.re_nsub+1, &pmatch, 0);
    pmesg(1, "The initial value of var_match is %i.\n", var_match);
    while (var_match == 0) // We are finding instances matching the regex
    {
        int length = pmatch.rm_eo-pmatch.rm_so;
        char* var_name = malloc(length*sizeof(char));
        strncpy(var_name, command_copy + start + pmatch.rm_so, length);
        pmesg(1, "The length is: %i - %i = %i.\n", pmatch.rm_eo, pmatch.rm_so, length);
        pmesg(1, "The variable's name is: %s.\n", var_name);
        free(var_name);
        start += pmatch.rm_eo;
        var_match = regexec(&comp_regex, command_copy+start, comp_regex.re_nsub+1, &pmatch, 0);
    }
    free(command_copy-start);
    return 0;
}

You are never modifying command_copy, and yet you are trying to free a location at command_copy-start.

Change the free(command_copy-start); line to read free(command_copy);.

The reason it works when the loop is never entered that because start never changes from zero.

I see a couple of problems in your code:

1. What is command? Using sizeof(command) will not give the string length of command; you need to malloc a buffer of size strlen(command) + 1 to strcpy from command into.

2. The statement in Zooba's answer about command_copy.

In the future, would you consider using code formatting? Just one example of coding style:

int parse_variables(size_t read_len)
{
    regex_t         comp_regex;
    regmatch_t      pmatch;
    int             start = 0;
    char*           command_copy;
    int             var_match;
    Some_result_t   regcomp_result;

    command_copy = malloc(sizeof(command));
    strcpy(command_copy, command);

    regcomp_result = regcomp (&comp_regex,               /* <description of what this parameter does> */
                              "[$][^0-9_][A-Z0-9_]+",    /* <description of what this parameter does> */
                              REG_EXTENDED);             /* <description of what this parameter does> */

    if (regcomp_result != 0 )
    {
        pmesg(1, "Regex compilation failed. Not parsing for variables.\n");
        return -1;
    }

    var_match = regexec (&comp_regex,                    /* <description of what this parameter does> */
                         command_copy + start,           /* <description of what this parameter does> */
                         comp_regex.re_nsub + 1,         /* <description of what this parameter does> */
                         &pmatch,                        /* <description of what this parameter does> */
                         0);                             /* <description of what this parameter does> */

    pmesg (1,
           "The initial value of var_match is %i.\n",
           var_match);

    while (var_match == 0) // We are finding instances matching the regex
    {
        int    length;
        char*  var_name;

        length = pmatch.rm_eo-pmatch.rm_so;
        var_name = malloc(length * sizeof(char));

        strncpy (var_name,
                 command_copy + start + pmatch.rm_so,
                 length);

        pmesg (1,
               "The length is: %i - %i = %i.\n",
               pmatch.rm_eo,
               pmatch.rm_so,
               length);

        pmesg (1,
               "The variable's name is: %s.\n",
               var_name);

        free(var_name);
        start += pmatch.rm_eo;

        var_match = regexec (&comp_regex,
                             command_copy+start,
                             comp_regex.re_nsub+1,
                             &pmatch,
                             0);

    } /* while (var_match == 0) */

    free(command_copy-start);
    return 0;
}

Apart from code style, the code functionality is identical to the original, though coverted from C++ to C. Variable declarations was written in C++.

I haven't done the mental gymnastics to figure it out but it should be trivial to run through this code using the debugger. What address is returned from malloc(), and what is being passed to free()?

Also, I consider it bad form to pass a calculated pointer to free(). You should be passing the same value that was returned from malloc(). Create another variable if needed.

Note that strncpy() doesn't do what you appear to believe it does. If the string length of the source is the same as the number of characters available in the destination, then the resulting string is not null-terminated. This is almost never what you want.

You can fix this by doing the following:

     char* var_name = malloc((length + 1)*sizeof(char));
     strncpy(var_name, command_copy + start + pmatch.rm_so, length);
     var_name[length] = 0;