Memory allocation: recommended practices

Question

I'm rambling a little with hacker 2 (the passwords') and, in the meantime, gained some insight about pointers, mem-alloc, 'valgrind', etc... It's been a truly great, challenging experience.

But, while I started digging (just a tiny little bit) deeper into these subjects, I started to feel like I need to learn (well.. a lot) how to better structure my programs, control the flow, and the memory allocation and "free()".

So, as a general 'styleguide-like' question: which are some (basic) recommended practices while allocating (calloc/malloc) and freeing memory?

E.g.: I wrote a few functions in hacker2 that deal, most of them, with strings (declared as char *). Since I (and I admit it =P) was kind of improvising a little as needed

(and there you have your first style-guide:

don't ever improvise!

;),

I decided to calloc() mem for most of them inside the function that actually uses them.

But then, if I return such char*, neither can I free it inside the function where I allocated it, nor in main(). Therefore, I get LEAK SUMMARY: definitely lost [...] in valgrind.

So, a general rule could be:

Don't return pointers. Pass them in as arguments, "fill" them with data inside your functions, and free them back in main() when you're certain you won't need them anymore.

Do I have a point? Or is this thought completely misleading?

From such rule, a second thought follows:

Declare and calloc() (and, as stated, eventually free()) all your outer variables in main(). Only declare and calloc inside functions those inner variables that are discarded as soon as you leave the function's scope (because they contain auxiliar data).

This is all related to another "strategy" I used in my program: as the flow would get more complex (in terms of 'CS50' complexity, that is.. ;) I tried to evaluate (inside each function) different errors (mem alloc, I/O, pass not found...) with different return values.

Then, back in main(), would I use such values to determine how & why is my program ending, and print out appropriate user messages, and end my program by returning different values.

E.g. (in main()):

retUnhash = unhash(passwd, cyph, salt, METHOD);

if (retUnhash == 1)
{
    printf("%s\n", "ERROR: I/O error.");
    return 3;
}
else if (retUnhash == 2)
{
    printf("%s\n", "=(");
    printf("%s\n", "Sorry, we couldn't guess your password.");
    return 4;
}
else if (retUnhash == 3)
{
    printf("%s\n", "ERROR: unknown method name.");
    return 5;
}

or the like...

But, if I follow such strategy, I risk returning main() and exiting my program before I get to free all my previously allocated mem-chunks, therefore getting all those ugly definitely lost signs from valgrind.

I could either:

• free() all the variables in every brach of the else if construct (mmm... don't sound too good =P)

• don't actually return on each brach, but set a return_value (and optionally a return_message), and at last, after freeing everything, return as in:

  printf("%s\n", return_message); return return_value;

So, another "rule" derives:

ALWAYS return ONCE in your code.

Does any of this make any sense to anyone? Are there good, compact, begginers' oriented styleguides regarding code structuring? (I feel like this might be a dumb question: there has to be such thing ;)

Or is it just something you eventually get a hang of after writing tons of lines of code?

Is there any way we can avoid getting bad coding habits and learn "the right way to do it" from the very beginning?

Answer 1

I'm not sure you needed to learn about pointers and memory management in order to solve hacker 2. I think you've gone a little bit far. However, I'm gonna try answering your questions.

A pointer is basically a variable that stores an address of a location in memory. Pretty much every type has its own pointer type. For example, char has char *, int has int *, struct s has struct s *, etc.

You'll learn more about pointers, what they can point to and how to access the data that they point to as you proceed with the course.

The function calloc allocates a piece of memory on the heap and returns the address of this memory location.

Allocated memory on the heap has to be freed manually (by calling free) as it had to be allocated manually. If your program doesn't free the memory it allocates on the heap, it will risk leaking the memory.

But then, if I return such char*, neither can I free it inside the function where I allocated it, nor in main(). Therefore, I get LEAK SUMMARY: definitely lost [...] in valgrind.

The definitely lost bytes are memory that you could never free because you lost access to it.

For example, if I allocated memory on the heap using calloc which returned the address 0x123 that I stored into a pointer. Then, in the middle of my program, I reassigned this pointer to another address 0x456. At this point, if I don't have another pointer that stores address 0x123, I could never free the memory at this location since I don't have access to it anymore.

Another example, would be having a pointer that has memory allocated for it, but this pointer goes out of scope at some point leaving me with no other way to access the memory that was allocated for it in order to free it.

Don't return pointers. Pass them in as arguments, "fill" them with data inside your functions, and free them back in main() when you're certain you won't need them anymore.

I don't see any reason why not returning pointers. A lot of functions in the standard library do return pointers. It's just that you should consider freeing any memory that you allocate on the heap as soon as you're completely done using them not to risk leaking the memory as I showed earlier.

And obviously if a function returns an address (a pointer), you can free the memory that was allocated for this pointer (if any) by calling free on it outside the function. For example, given this silly example

int *allocateMemoryForAnInt(void)
{
    return (malloc(sizeof (int)));
}

// outside the function
int *ptr = allocateMemoryForAnInt();

// After I'm done using ptr
// make sure ptr is not NULL
if (ptr)
    free(ptr);

Keep in mind though that if a pointer x and a pointer y point to the same memory location, then calling free on either of them is sufficient for freeing the memory that they point to. Calling free on both while they're still pointing to the same memory location is a run time error. So make sure you're not freeing a pointer inside of a function and outside of it or something!

And again, you don't have to declare all your pointers, allocate memory for them and/or free the allocated memory in main. You can do that pretty much anywhere you need.

As for your code, there are many approaches to avoid duplicating code in such case (i.e., if you need to call free on a bunch of pointers in main before returning). Here's one approach, assuming myFunc returns a code that indicates whether the function successfully accomplished its task and errorCode is a function that tests whether the passed code is an error code

int code = myFunc();

if (errorCode(code))
    // free pointers
    // handle each error code appropriately

ALWAYS return ONCE in your code.

I really don't know where you come up with these rules from :D

The basic rule is that you do what you need. I don't think you should expect someone to agree/disagree with you on this. Each problem is different.

Is there any way we can avoid getting bad coding habits and learn "the right way to do it" from the very beginning?

Yes, by searching for the good practices regarding something and watching other people's clean code. There are also books that teach you how to design program and how to write clean code, but I can't recommend any since I never read any yet.

I think you shouldn't care about these things that much right now and rather you should focus on the fundamentals and how to get your problems solved as well as doing your best making your code easy to read and understand at the moment.

Then, at some point (in the future), you'll need to care more about these issues and maybe read a couple of books about it. Your programs are gonna get way larger and more complex than crack ;)