views:

171

answers:

8

I am working with my first straight C project, and it has been a while since I worked on C++ for that matter. So the whole memory management is a bit fuzzy.

I have a function that I created that will validate some input. In the simple sample below, it just ignores spaces:

int validate_input(const char *input_line, char** out_value){

    int ret_val = 0; /*false*/
    int length = strlen(input_line);
    out_value =(char*) malloc(sizeof(char) * length + 1);

    if (0 != length){

        int number_found = 0;
        for (int x = 0; x < length; x++){

            if (input_line[x] != ' '){ /*ignore space*/

                /*get the character*/
                out_value[number_found] = input_line[x];
                number_found++; /*increment counter*/
            }
        }
        out_value[number_found + 1] = '\0';

        ret_val = 1;
    }

    return ret_val;

}

Instead of allocating memory inside the function for out_value, should I do it before I call the function and always expect the caller to allocate memory before passing into the function? As a rule of thumb, should any memory allocated inside of a function be always freed before the function returns?

+2  A: 

You should free that memory before the function returns in your above example. As a rule of thumb you free/delete allocated memory before the scope that the variable was defined in ends. In your case the scope is your function so you need to free it before your function ends. Failure to do this will result in leaked memory.

As for your other question I think it should be allocated going in to the function since we want to be able to use it outside of the function. You allocate some memory, you call your function, and then you free your memory. If you try and mix it up where allocation is done in the function, and freeing is done outside it gets confusing.

Ben Burnett
Mixing where the allocation/freeing is done doesn't make sense to me, thanks for the post. I am just not sure it is practiced very much. I just noticed simple functions like strtol for example take a char** and allocate the memory for you inside the function.
Keith P
@Keith P: `strtol()` doesn't allocate memory for you - the `char **` is used to update a pointer you supply, but only to point to values within the string you also supplied. `strtok()` is similar; in fact, the only standard functions that return a pointer you have to later free are `malloc()`, `calloc()` and `realloc()`. Some non-standard but common functions, however, also do this - like `strdup()`.
caf
@caf, Thanks for that clarification! I was looking at some code that didn't seem like it was allocating memory before passing it to strtol, but maybe I missed the allocation earlier in the pipeline of the app.
Keith P
A: 

A typical practice is to allocate memory outside for out_value and pass in the length of the block in octets to the function with the pointer. This allows the user to decide how they want to allocate that memory.

One example of this pattern is the recv function used in sockets:

 ssize_t recv(int socket, void *buffer, size_t length, int flags);
Doug T.
A: 

In this example you should be neither freeing or allocating memory for out_value. It is typed as a char*. Hence you cannot "return" the new memory to the caller of the function. In order to do that you need to take in a char**

In this particular scenario the buffer length is unknown before the caller makes the call. Additionally making the same call twice will produce different values since you are processing user input. So you can't take the approach of call once get the length and call the second time with the allocated buffer. Hence the best approach is for the function to allocate the memory and pass the responsibility of freeing onto the caller.

JaredPar
Good points Jared, but I do know the size of "input_line" before calling the function. How does the caller always know that they have to free the memory in your scenario?
Keith P
+2  A: 

The idea of whether the function/module/object that allocates memory should free it is somewhat of a design decision. In your example, I (personal opinion here) think it is valid for the function to allocate it and leave it up to the caller to free. It makes it more usable.

If you do this, you need to declare the output parameter differently (either as a reference in C++ style or as char** in C style. As defined, the pointer will exist only locally and will be leaked.

Mark Wilkins
Thanks for the tip on the char**! Although, if I know the memory size of "input_line" before I call the function, doesn't it make sense to allocate before calling?
Keith P
@Keith: I think it depends on the usage scenarios and what works best in your application. In reality, you probably are likely going to reduce the chance of memory leaks if the caller allocates and frees it (that type of code is easier to verify in code walkthroughs for example). A code walkthrough of a function that returns allocated memory cannot detect the leaks of callers. But you could argue that allocating it in the function makes sense if you want to be "frugal". It could compute the true needed size first and allocate only as much as needed. Just a thought.
Mark Wilkins
+6  A: 

I follow two very simple rules which make my life easier.

1/ Allocate memory when you need it, as soon as you know what you need. This will allow you to capture out-of-memory errors before doing too much work.

2/ Every allocated block of memory has a responsibility property. It should be clear when responsibility passes through function interfaces, at which point responsibility for freeing that memory passes with the memory. This will guarantee that someone has a clearly specified requirement to free that memory.

In your particular case, you need to pass in a double char pointer if you want the value given back to the caller:

int validate_input (const char *input_line, char **out_value_ptr) {
    : :
    *out_value_ptr =(char*) malloc(length + 1); // sizeof(char) is always 1
    : :
    (*out_value_ptr)[number_found] = input_line[x];
    : :

As long as you clearly state what's expected by the function, you could either allocate the memory in the caller or the function itself. I would prefer outside of the function since you know the size required.

But keep in mind you can allow for both options. In other words, if the function is passed a char** that points to NULL, have it allocate the memory. Otherwise it can assume the caller has done so:

    if (*out_value_ptr == NULL)
        *out_value_ptr =(char*) malloc(length + 1);
paxdiablo
I like that option, don't assume. Check to see if it is null, if not allocate.
Keith P
If a function allocates some memory, isn't the normal practice to return a pointer to that memory block (instead of using **ptr)? Just as malloc() itself does. Creating a function that may either allocate or not allocate memory just makes things unnecessarily complex.
PauliL
@Paul: It depends. Sometimes you need to returns an indicator of success or failure and you can often overload NULL as an error condition. But sometimes you may want to return a value and a pointer and passing in a pointer is the usual way of doing that - in other words, use the return value for one thing, a pointer to something to set for the other.
paxdiablo
+1  A: 

Here are some guidelines for allocating memory:

  1. Allocate only if necessary.
  2. Huge objects should be dynamically allocated. Most implementations don't have enough local storage (stack, global / program memory).
  3. Set up ownership rules for the allocated object. Owner should be responsible for deleting.

Guidelines for deallocating memory:

  1. Delete if allocated, don't delete objects or variables that were not dynamically allocated.
  2. Delete when not in use any more. See your object ownership rules.
  3. Delete before program exits.
Thomas Matthews
What do you mean "Owner should be responsible for deleting"? In the scenario described above, who is the owner?
Keith P
In your example the caller would be the owner. Once the function returns, it's passing ownership to the caller since it no longer has a pointer to the memory it allocated.
munificent
A: 

First, this code example you give is not ANSI C. It looks more like C++. There is not "<<" operator in C that works as an output stream to something called "cout."

The next issue is that if you do not free() within this function, you will leak memory. You passed in a char * but once you assign that value to the return value of malloc() (avoid casting the return value of malloc() in the C programming language) the variable no longer points to whatever memory address you passed in to the function. If you want to achieve that functionality, pass a pointer to a char pointer char **, you can think of this as passing the pointer by reference in C++ (if you want to use that sort of language in C, which I wouldn't).

Next, as to whether you should allocate/free before or after a function call depends on the role of the function. You might have a function whose job it is to allocate and initialize some data and then return it to the caller, in which case it should malloc() and the caller should free(). However, if you are just doing some processing with a couple of buffers like, you may tend to prefer the caller to allocate and deallocate. But for your case, since your "validate_input" function looks to be doing nothing more than copying a string without the space, you could just malloc() in the function and leave it to the caller. Although, since in this function, you simply allocate the same size as the whole input string, it almost seems as if you might as well have the caller to all of it. It all really depends on your usage.

Just make sure you do not lose pointers as you are doing in this example

BobbyShaftoe
Equally good reasoning can be found on "who needs memory, in terms of final use, does the allocation and is responsible for freeing". It's not saying "then, it's always caller". From experience, it's a coding practice that will find less restriction than "callee alloc, caller frees, or caller both".Imagine this same code snippet (correctly) returning out_value ONLY if spaces are found.
jpinto3912
'There is not "<<" operator in C' ??? I think you'll find there is :-) Just not in the context of `cout`.
paxdiablo
@paxdiablo, good catch I meant to qualify that.
BobbyShaftoe
A: 

Some rough guidelines to consider:

  1. Prefer letting the caller allocate the memory. This lets it control how/where that memory is allocated. Calling malloc() directly in your code means your function is dictating a memory policy.
  2. If there's no way to tell how much memory may be needed in advance, your function may need to handle the allocation.
  3. In cases where your function does need to allocate, consider letting the caller pass in an allocator callback that it uses instead of calling malloc directly. This lets your function allocate when it needs and as much as it needs, but lets the caller control how and where that memory is allocated.
munificent