c pointer, how to free it/them into a function

Question

This is my code:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void getinfo(unsigned int a, unsigned int b, char **pStr);

int main(){
    unsigned int len_max = 8;
    unsigned int current_size = 0;
    current_size = len_max;
    char *host, *user;
    char *pStr = malloc(len_max);
    if(pStr == NULL){
        perror("\nMemory allocation\n");
        return EXIT_FAILURE;
    }
    printf("Inserisci hostname: ");
    getinfo(len_max, current_size, &pStr);
    if((host=malloc(strlen(pStr)+1 * sizeof(char))) == NULL) abort();
    strncpy(host, pStr, strlen(pStr)+1);
    printf("Inserisci username: ");
    getinfo(len_max, current_size, &pStr);
    if((user=malloc(strlen(pStr)+1 * sizeof(char))) == NULL) abort();
    strncpy(user, pStr, strlen(pStr)+1);
    printf("\nHostname: %s\nUsername: %s\n", host, user);
    free(pStr);
    free(host);
    free(user);
    return EXIT_SUCCESS;
}

void getinfo(unsigned int a, unsigned int b, char **pStr){
    unsigned int i = 0;
    int c = EOF;
    while((c = getchar()) != '\n'){
        (*pStr)[i++] = (char)c;
        if(i == b){
            b = i+a;
            if((*pStr = realloc(*pStr, b)) == NULL){
                perror("\nMemory allocation error\n");
                exit(EXIT_FAILURE);
            }
        }
    }
    (*pStr)[i]='\0';
}

The problem is that if realloc fails i have to exit (because i cannot allocate memory). But before exit there's to free all the used pointers.
The problem is that if the function fails the first time there's only 1 pointer that have to be freed (pStr).
But if it fails the second time there's 2 pointers that have to be freed (pstr & user).
How can i fix it?

Answer 1

As already noted, if you are going to exit, then all practical modern O/S will release the allocated memory before exit. It was not always thus; early versions of AmigaDOS, IIRC, did not reclaim allocated memory automatically until reboot.

This is a simple case. There are more complex cases, such as you are parsing a file into memory and the 579^th memory allocation fails, and you'd like to release the previous 578 memory allocations so that the user can try again.

In such cases, you have to keep a record of each memory allocation that is relevant (which may itself require some memory allocation — though if you're parsing a file, you probably have a master structure which contains the complete description) and then release all the allocated data.

In your example, if this was not a main() function and if you did not abort on memory allocation error, then you would need to ensure that the three allocated pointers are released on exit from the function. The standard tricks for that include:

1. Initialize the pointers to 0 so they can be freed reliably:

   char *host = 0;
   char *user = 0;
   

2. When using realloc(), do not assign the result to the expression passed as the first parameter:

   Do NOT do:

   ptr = realloc(ptr, newsize);
   

   If (when) ptr is the only reference to the allocated memory and the reallocation fails, you've just leaked memory; there is no way to release the memory that is still allocated to you.

   Use:

   void *newptr = realloc(oldptr, newsize);
   if (newptr == 0)
       ...recover from memory allocation failure
   oldptr = newptr;
   

   The trouble with the simpler version is that you've just thrown away the only reference to the allocated memory. (Note that your code falls into the dangerous/leaking pattern).

3. Note that pretty much every function that acquires resources must either release the acquired resource before returning, or make the resource available to some other part of the program so that the other part can release the resource when it is done with it.

   The 'make available' operation might be returning the acquired resource (think of it as memory, but it could be a file descriptor, directory stream, or any of a large number of other allocated resources) to the calling function, or storing it in a structure that was passed to the current function, or copying it to a global or (file) static variable, or even stashing it in a (function) static variable so if the function is called again, it has some resource available on entry.

Answer 2

As a few people have pointed out, modern OS's reclaim memory on exit. However, it is considered a best practice to free your resources anyway, as this makes debugging easier. For example, if you are trying to find a leak and you use a tool like valgrind, all the memory you don't properly free (even if by the program logic, this doesn't matter) will appear as leaks. There are some large API's around that notoriously don't do this, and they make tracking leaks in applications which use them a nightmare.

Also, in some specialized environments it might be important to clean up after yourself. Therefore, it's a good habit to get into now.

A clean-up technique you'll see occasionally (eg, in the linux kernel) is something I think of as the "bail and release" pattern. It's one of the few (perhaps: only) contexts where goto is still considered acceptable. It depends upon you being able to free your resources in the opposite order you allocated them. Usually this is in the context of a single function, in this case main():

#include <stdlib.h>

int main(void) {
    int exit_status = EXIT_FAILURE;

    char *s1, *s2, *s3;

    s1 = malloc(100);
    if (!s1) return EXIT_FAILURE;

    s2 = malloc(100);
    if (!s2) goto endB;

    s3 = malloc(100);
    if (!s3) goto endA;

    exit_status = EXIT_SUCCESS;

    /* do whatever */

    free(s3);
endA:
    free(s2);
endB:
    free(s1);
    return exit_status;
}            

To explain: if allocating s1 fails, we just return -- there is nothing to clean-up. But if allocating s2 fails, we goto endB, freeing s1. If allocating s3 fails, we goto endA, which will free s2 and s1. Finally, if everything succeeded, we do whatever, and afterward, all three pointers will be freed. If this were a normal function, we might be returning a pointer, so there would be a separate return for that before the "end" bits, which would complete with "return null" instead.

Nb: please don't take this as a licence to make free-wheeling use of goto!

Answer 3

This is more of a general C language advice than a specific answer, but it's too long to go in comment.

The usual way to write C in the presence of dynamic resource management is to goto suitable labels which are followed by the relevant deallocation calls:

int f(int n)
{
    void * p1, * p2, * p3;
    int result = -1;

    p1 = malloc(n);

    if (!p1) goto END0;
    if (n % 2) goto END1;

    p2 = malloc(3 * n);

    if (!p2) goto END1;
    if (n % 7 == 0) goto END2;

    p3 = malloc(7 * n + 8);

    if (!p3) goto END2;

    if (do_useful_stuff(p1, p2, p3) == -1) goto END3;
    result = compute_more_stuff(p1, p2, p3);

END3:
    free(p3);
END2:
    free(p2);
END1:
    free(p1);
END0:
    return result;
}

The alternative is to split up your code into very small functions that do only one thing at a time and handle resource allocation in an ad-hoc fashion:

int small_function(void ** p)
{
    void * q = malloc(13);

    if (some_init(&q) == -1)
    {
        free(q);    // ad-hoc exit management
        return -1;
    }

    int n = complex_computation(q);
    free(q);
    return n;
}

Answer 4

Yeah, you don't have to free anything if this is the whole program.

The whole reason to free memory is so it can be reused somewhere later in the program. Even if your program went on from this point, you've only allocated a small number of bytes. It's OK to allocate them and keep them forever.

In fact, you don't even have to do the arithmetic you're doing there to carve out exact-sized mallocs, you could say Oh, usernames and hostnames are never more than like 30 chars, just to be sure I'll allocate 256 char blocks. Oh wait your max is 8 chars, whatever. Or even just make a global buffer 256 chars or 8 chars long. Then make sure your strncpy()s never go past len_max or else you're risking a buffer overflow hack.

meanwhile that getinfo() looks painful. Try something like fgets(mybuffer, len_max, stdin).

Last I checked, the executable doesn't even bother to 'free' all unfreed blocks at the end, it just walks away. The VM system returns all the used memory (including the stack and program code) to the OS, and the process vaporizes and it's over. The malloc()ed blocks are just a pattern of bytes on that memory, and it's all forgotten.

Answer 5

You don't have to free dynamically allocated memory before exiting. The OS will make all that memory available to the next process.