C - freeing memory allocated in a loop

Question

For the program I'm working on I need to perform "String Math" where, given two strings, I need to perform addition or subtraction along all the characters in the string to get a final result.

I'm having issues when it comes to freeing up pointers and feel I am really stuck when understanding how to use pointers in a loop.

Currently I have:

int main() {
    char * input = (char *)malloc(sizeof(char)*1024);
    char * stringA = (char *)malloc(sizeof(char)*64);
    char * stringB = (char *)malloc(sizeof(char)*64);
    char * result;
    char operand;
    int matched = 0;

    while(fgets(input, 1024, stdin) != NULL) {
        matched = sscanf(input, "%s %s %c", stringA, stringB, &operand);
        if (matched == 3) {
            if (strlen(stringA) < strlen(stringB)) {
                stringA = balanceLengths(stringA, stringB);
            } else if (strlen(stringA) > strlen(stringB)) {
                stringB = balanceLengths(stringB, stringA);
            }

            result = (char *)calloc(strlen(stringA)+2, sizeof(char));
            result = performStringMath(result, stringA, stringB, operand);

            printf("%s\n", result);

        }
    }

    //printf("%s\n%s\n", stringA, stringB);

    free(input);
    free(stringA);
    free(stringB);
    return 0;
}

And I'm really struggling to understand how I can free result on every iteration of the loop, because if I were to add a free(result) after the printf statement I get several invalid read and write errors from valgrind, I also appear to get similar errors if I assign memory for calloc before the loop.

Other methods:

    char * performStringMath(char *result, char *stringA, char *stringB, char operand) {
    if (operand == '+') {
        return performAddition(result, stringA, stringB);
    } else if (operand == '-') {
        return performSubtraction(stringA, stringB);
    } else {
        return 0;
    }
}

char * balanceLengths(char *shorter, char *longer) {
    int toAdd, i;
    toAdd = (int)(strlen(longer) - strlen(shorter));
    char *retString = (char *) malloc(sizeof(char) * strlen(longer)+ toAdd);

    for (i = 0; i < toAdd; i++) {
        retString[i] = '0';
        retString[toAdd+i] = shorter[i];
    }
    return retString;
}

char * performSubtraction(char *stringA, char *stringB) {
    return "";
}

char * performAddition(char *result, char *stringA, char *stringB) {
    int carry, sum, i;
    sum = 0;
    carry = 0;
    for (i = (int)strlen(stringA)-1; i >= 0; i--) {
        sum = (stringA[i]-48) + (stringB[i]-48) + carry;
        carry = 0;
        if (sum >= 10) {
            sum = sum-10;
            carry = 1;
        }
        stringA[i] = (char)(sum+48);
    }

    if (carry != 0) {
        result[0] = '1';
        result = strcat(result, stringA);
    } else {
        result = stringA;
    }

    return result;
}

I've actually resolved the issue by replacing

result = stringA;

to result = strcpy(result, stringA);

in function perform addition.

Welcome to Stack Overflow! [Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Mar 27 '17 at 04:27
What is the problem with adding `free(result);` as the last statement of the `while` loop body? — Sourav Ghosh, Mar 27 '17 at 04:28
You don't "free a pointer". You free the object it points to! — too honest for this site, Mar 27 '17 at 04:32

score 3 · Answer 1 · answered Mar 27 '17 at 04:29

Your code doesn't look like you're very confused. It looks mostly right. I see a single missing call to free(). In your loop, you call calloc(), but never call free() on that memory. If you aren't going to use result after you print it out , then you can free it once it's printed:

        result = (char *)calloc(strlen(stringA)+2, sizeof(char));
        result = performStringMath(result, stringA, stringB, operand);

        printf("%s\n", result);
        free(result);

The other strings could be allocated and freed on every iteration of the loop, but it would make the program run more slowly and not help in any way.

If that's giving you errors with a tool like valgrind, then it could be that something's going wrong in performStringMath(). Without the code to that function, though, we can't really say what it could be.

Thanks for the reply, I've edited the original post with the remaining methods. When I add the free(result) you mentioned valgrind spits out a bunch of invalid write and reads as well as an invalid free on the free(result) line. I feel like I'm performing free correctly on the pointers but I can't seem to get this working. — Raetro, Mar 27 '17 at 04:55

C - freeing memory allocated in a loop

1 Answers1