[Solved] C error handling

Nanthiel · 2011-10-02 12:39:12

Hello everyone!

I would like to know how to handle errors cleanly and robustly in C for functions of this form:

/* Inside some function.  */
void *ptr_one = malloc(...);
if (!ptr_one)
  return some error value;

void *ptr_two = malloc(...);
if (!ptr_two) {
  free(ptr_one);
  return some error value;
}

Is there a better way to do this?

Now the second problem is, what if the first malloc() call is a realloc() call? How would one revert to the program's previous state? The same goes for changing a bunch of variables before the error occurs. How does one "fix" that? Should I just abort() even though I'm implementing an Abstract Data Type? Should I re-realloc stuff and have old_<var> variables to restart the new ones? I assume ADT's should never interrupt a program's execution, but report errors to the caller. Should I free the whole data structure and tell the caller "better luck next time", resulting in maybe-critical data loss?

This kind of problem is actually exactly the same whether using exceptions or return values, or using any imperative language, so I think the answer is important. I've searched the forums and Google, but I don't seem to find recommendations on this sort of thing, and neither has anyone at university ever mentioned this (and I'm starting my third/last year now --- they sure are making quality programmers).

Thanks for your suggestions,
Nanthiel

Last edited by Nanthiel (2011-10-03 12:50:25)

falconindy · 2011-10-02 13:24:14

From a library perspective, it's always better to try and recover as much as possible from an error -- just report it to the front end to do as it wishes. I don't believe aborting (via abort or otherwise) is ever the right thing to do. Your example is fine... you might find labels useful as a "bail out" strategy if you're allocating a lot from within a single function (or you might consider refactoring).

As far as realloc goes, the "safe" way to handle it is to create a new pointer for the return of realloc. If realloc returns NULL, then you return the original pointer (as it isn't modified in any way on failure). It'll be up to the caller to check errno for ENOMEM to know if the returned address is unchanged or resized.

In reality, malloc/realloc failure is extremely rare in Linux due to memory overcommit. You'll likely run into your OOM situation later on when the memory is actually written to (and when the kernel finally allocates the pages).

Cloudef · 2011-10-02 13:34:36

To add to falconindy's post, here is example of realloc:

/* needs realloc */
void *ptr2 = realloc( ptr, items * size );
if(!ptr2) {
   /* realloc fails, try to allocate more and copy the old memory */
   ptr2 = calloc( items, size );
   if(!ptr2)
      return( ptr ); /* allocating failed, bail out and return the old pointer */
   /* allocating success, copy the old stuff */
   memcpy( ptr2, ptr, old_items * size );
   free( ptr ); ptr = ptr2;
}
else
   ptr = ptr2; /* realloc success, swap */
return( ptr );

Something like that, not tested however
You might also pass the modifiable pointer as argument and return if the reallocating succeed, instead of returning pointer and not knowing what happened.

Last edited by Cloudef (2011-10-02 13:38:03)

Google · 2011-10-02 15:39:54

I would also be careful if you are doing any kind of multiple-allocs and trying to handle errors after an earlier malloc. It's easy to bail out early and forget to free previous mallocs. <== memory leak

Last edited by Google (2011-10-02 15:40:53)

falconindy · 2011-10-02 17:05:03

Cloudef wrote:

To add to falconindy's post, here is example of realloc:
/* needs realloc */
void *ptr2 = realloc( ptr, items * size );
if(!ptr2) {
   /* realloc fails, try to allocate more and copy the old memory */
   ptr2 = calloc( items, size );
   if(!ptr2)
      return( ptr ); /* allocating failed, bail out and return the old pointer */
   /* allocating success, copy the old stuff */
   memcpy( ptr2, ptr, old_items * size );
   free( ptr ); ptr = ptr2;
}
else
   ptr = ptr2; /* realloc success, swap */
return( ptr );
Something like that, not tested however
You might also pass the modifiable pointer as argument and return if the reallocating succeed, instead of returning pointer and not knowing what happened.

There's a few problems with this -- mainly that you're leaking memory on realloc fail (as it returns NULL, but doesn't modify the first argument). Additionally, if realloc fails, there's almost no way calloc will succeed -- calloc demands that the kernel maps the pages right then and there, since it zeroes out the memory (and thus accesses the entire allocation). You're also trying to outsmart the malloc library and assume that isn't trying to resize in place, falling back on a malloc/memcpy/free(old) (which I believe it does).

I was thinking of something much simpler:

void *xrealloc(void *ptr, size_t newsz)
{
	void *origptr = ptr;

	errno = 0;
	ptr = realloc(ptr, newsz);
	if(!ptr) {
		fprintf(stderr, "error: insufficient memory\n");
		return origptr;
	}

	return ptr;
}

int main(void) {
	void *data;

        data = malloc(2000);
        if(!data) {
                fprintf(stderr, "uh oh!\n");
                return 1;
        }

        /* do stuff with data, eventually you need to resize */

	data = xrealloc(data, 40000000);
	if(errno == ENOMEM) {
		fprintf(stderr, "uh oh!\n");
	}

	/* other stuff happens */

	free(data);

	return 0;
}

Last edited by falconindy (2011-10-02 17:05:45)

Cloudef · 2011-10-02 17:35:08

falconindy wrote:

There's a few problems with this -- mainly that you're leaking memory on realloc fail (as it returns NULL, but doesn't modify the first argument). Additionally, if realloc fails, there's almost no way calloc will succeed -- calloc demands that the kernel maps the pages right then and there, since it zeroes out the memory (and thus accesses the entire allocation). You're also trying to outsmart the malloc library and assume that isn't trying to resize in place, falling back on a malloc/memcpy/free(old) (which I believe it does).

Nope, there should be no leak. It returns the old pointer if calloc fails, and frees if it succeeds (and swaps the new one). But in real use scenario there should be also return for fail/success. I read somewhere (prob stackoverflow) that realloc can fail on some systems if the memory can't be realloced to contiguous, that's when you do malloc/calloc and memcpy.

Last edited by Cloudef (2011-10-02 17:44:55)

Nanthiel · 2011-10-02 17:38:28

As far as I know, realloc() behaves like malloc()+memcpy() when it can't extend/shrink its current block of memory to the desired size, and does so automatically. So trying to malloc/calloc + memcpy after it doesn't make sense. But naturally, implementations may differ.

I've never thought of using `errno', and will probably use it more.

However, my main problem, and sofar unsolved, is when using multiple previous allocations, especially re-allocations, and then having something else in the function go wrong, like a next malloc.
I could, sure, swap them around and perform the malloc earlier, but what if I can't? Should I re-allocate the data back to its (probably smaller) size? How would I make the function recover? And what if so far the function changed several variables? Should I keep their old values 'till the end and swap them there, when I know no errors will occur? This makes the code very messy and adds many operations.

The easiest solution would be to forget about realloc() altogether and always use malloc()+memcpy(), but realloc() performs faster. I would then memcpy() at the end, when I know allocations were successful.

What do you think?

stqn · 2011-10-03 00:53:31

In C I was using goto to cleanup in case of errors.

void blah( void ) {
  void *p1 = NULL, *p2 = NULL;
  p1 = malloc( 1000 );
  if( !p1 ) return;
  p2 = malloc( 2000 );
  if( !p2 ) goto error;
...
error:
  free(p1);
  free(p2);
}

... or something similar.

Edit: Oh BTW it's pointless to check for success in memory allocations under Linux, since they will always succeed (or your program will be killed.) But if you want to write cross-platforms apps, it's probably better to check...

Last edited by stqn (2011-10-03 00:56:04)

Nanthiel · 2011-10-03 11:48:01

stqn wrote:

In C I was using goto to cleanup in case of errors.
void blah( void ) {
  void *p1 = NULL, *p2 = NULL;
  p1 = malloc( 1000 );
  if( !p1 ) return;
  p2 = malloc( 2000 );
  if( !p2 ) goto error;
...
error:
  free(p1);
  free(p2);
}
... or something similar.
Edit: Oh BTW it's pointless to check for success in memory allocations under Linux, since they will always succeed (or your program will be killed.) But if you want to write cross-platforms apps, it's probably better to check...

About your app being killed: that is not exactly correct. There is a function defined in the kernel which evaluates processes when an "out of memory" error occurs (the error means the virtual address space is exhausted). The process with the highest score gets killed. It may not be the one that caused it. Think of it like this: maybe your process took the rest of the memory, and the next time something like DBus calls malloc, it fails. Will the kernel kill DBus? Not really, since it's been around for a long time, which is one of the evaluation factors. I'm not an expert, but I presume Google would know more.

Letting any errors go unnoticed by a program is considered extremely bad practice and should be avoided. Also, cleanly handling errors is important if other applications depend on your code. If your error handling simply does exit() or abort(), you may interrupt other applications. An example of this is Pulse Audio (as I've read in some blog), which simply exit()'s if a memory error occurs. Meaning any application using audio will probably be killed with it. If it exited cleanly, the application would run normally, but without sound.

Maybe they will change Linux memory handling at some point, and that's when many programs ignoring checking for this may break.

My current solution to my problem is that I managed to rearrange the code, so that the realloc call occurs before the malloc call. So if the malloc goes wrong, I won't have to realloc to the previous size to reset the state. Luckily, I only have 1 realloc in the function.

Another problem would be if the code modified (especially deleted) a part of the data structure before the error occurs. I have no idea how to correct that rather than to make copies before any changes, and that's just incredibly inefficient. So so far I still have no solution to handling of such errors, except re-thinking the entire problem so this isn't necessary, when possible, or just copying anything in advance that might get changed.

Is there truly no magic solution?

Barrucadu · 2011-10-03 12:20:53

The way I account for errors is to assert function preconditions (and often postconditions), then test the functions and fix things where they fail - with this, the functions are "guaranteed" to work. Of course, that's only good for logic errors (eg free being passed a NULL pointer), for user errors I print an error message and call some clean-up routine. However, I haven't written any libraries, other than to be used in my own personal projects, so it's possible I'd use a different approach in the case where the code will be used by many people.

As an example, here are the memory functions I use:

utils/memory.h:

#ifndef __SHARED_MEMORY_H
#define __SHARED_MEMORY_H

#define xalloc(T)     (T*) xalloc_internal (sizeof (T))
#define xallocn(T, N) (T*) xalloc_internal (sizeof (T) * (size_t)N)
#define xfree(P)      xfree_internal((void**)&P)

/**                                                                                                                                                                                                                                                                               
 * Allocate, check, and zero some memory                                                                                                                                                                                                                                          
 *                                                                                                                                                                                                                                                                                
 * @param The size of memory to allocate                                                                                                                                                                                                                                          
 */
void*
xalloc_internal (size_t size);

/**                                                                                                                                                                                                                                                                               
 * Free a pointer and set it to NULL                                                                                                                                                                                                                                              
 *                                                                                                                                                                                                                                                                                
 * @param Pointer to the pointer to free                                                                                                                                                                                                                                          
 */
void
xfree_internal (void** pointer);

#endif

utils/memory.c

#include <stdlib.h>
#include <assert.h>
#include "memory.h"

/**                                                                                                                                                                                                                                                                               
 * Allocate, check, and zero some memory                                                                                                                                                                                                                                          
 */
void*
xalloc_internal (size_t size)
{
  void *out;

  out = calloc (1, size);

  assert (out != NULL);

  return out;
}

/**                                                                                                                                                                                                                                                                               
 * Free a pointer and set it to NULL                                                                                                                                                                                                                                              
 */
void
xfree_internal (void** pointer)
{
  assert (pointer  != NULL);
  assert (*pointer != NULL);

  free (*pointer);

  *pointer = NULL;
}

This is a design decision, but I consider malloc failing an internal logic error, as it shouldn't fail on Linux due to the memory allocation strategy, and so a failure indicates something may be wrong with the libc. Were I writing code to be used by other people, I would probably not just bail out on a malloc fail.

Simply asserting preconditions and postconditions reduces my error handling code significantly, and I don't consider aborting on a logic error to be a problem, as such a thing, like a segfault, indicates a bug in the code and so it "shouldn't" arise with proper testing. However, things like the user giving a bad filename, or trying to connect to an IP that doesn't exist - any errors which the user could have caused - do get handled gracefully.

Last edited by Barrucadu (2011-10-03 12:31:31)

Nanthiel · 2011-10-03 12:50:08

Barrucadu: I agree with you completely, and I started to use asserts the same way (for logic errors, not mallocs --- as in, preconditions, postconditions and/or invariants). Kind of something I stole from the Eiffel languge when I was looking into it. But I still want to handle mallocs properly, just to feel better, even if it isn't necessary. I'm currently programming for the sake of programming, not for some special reason. Learning new languages and the like. Which I think will help me greatly once I do start working for real.

So currently the only "solution(s)" I can think of are the ones at HERE. And it makes sense, I guess. Having to change something that has been done cannot ever be easy. So I'm marking this thread [Solved] and am going to be more careful structuring my code so these things won't be necessary.

I hope others will find this thread useful too. There's lots of useful information going around: using the goto statement for cleaner error handling, using asserts to test for logic errors (that should never happen) and the many uses of errno!

Last edited by Nanthiel (2011-10-03 12:51:02)

stqn · 2011-10-03 13:30:31

Nanthiel wrote:

About your app being killed: that is not exactly correct. There is a function defined in the kernel which evaluates processes when an "out of memory" error occurs (the error means the virtual address space is exhausted). The process with the highest score gets killed.

Yes, but if it's not your program that gets killed, then your allocation will succeed . (At least I believe that's how it works.)

Anyway I wasn't really suggesting that you stop checking for out of memory in your programs... That was more of a rant.

Nanthiel · 2011-10-03 17:04:36

stqn wrote:

Yes, but if it's not your program that gets killed, then your allocation will succeed . (At least I believe that's how it works.)
Anyway I wasn't really suggesting that you stop checking for out of memory in your programs... That was more of a rant.

D'oh! I must be stupid. You're absolutely correct (most probably). That makes sense. >_<

tavianator · 2011-10-04 04:26:33

If you want to understand how Linux handles memory allocation failures, read the NOTES section under "man malloc".

Basically, by default, most allocations will succeed even if there's no memory available (malloc(1ULL << 63ULL) will probably still return NULL though). Memory is actually allocated as it's accessed by your application, and in the OOM condition your program will probably be killed.

This behaviour is controlled by /proc/sys/vm/overcommit_memory, and it's quite possible for it to be disabled. So, ALWAYS check the return value of malloc(), either with an xmalloc()-type function that exit()s when malloc() fails, or by testing it everywhere in your code. I prefer to bail out on malloc() failures, but it depends on the application.

And yes, goto is basically the best solution I'm aware of for the pattern the OP is describing. But you seem to want to write code with the "strong guarantee", which isn't always possible. It may be easier to try for the weak guarantee.

Nanthiel · 2011-10-04 06:46:35

tavianator wrote:

...

Thank you for the clarification on malloc().

Yes I am going for the "strong guarantee". I might not on a real project, but for this academic stuff, going all the way is beneficial for the learning experience.

Arch Linux

#1 2011-10-02 12:39:12

[Solved] C error handling

#2 2011-10-02 13:24:14

Re: [Solved] C error handling

#3 2011-10-02 13:34:36

Re: [Solved] C error handling

#4 2011-10-02 15:39:54

Re: [Solved] C error handling

#5 2011-10-02 17:05:03

Re: [Solved] C error handling

#6 2011-10-02 17:35:08

Re: [Solved] C error handling

#7 2011-10-02 17:38:28

Re: [Solved] C error handling

#8 2011-10-03 00:53:31

Re: [Solved] C error handling

#9 2011-10-03 11:48:01

Re: [Solved] C error handling

#10 2011-10-03 12:20:53

Re: [Solved] C error handling

#11 2011-10-03 12:50:08

Re: [Solved] C error handling

#12 2011-10-03 13:30:31

Re: [Solved] C error handling

#13 2011-10-03 17:04:36

Re: [Solved] C error handling

#14 2011-10-04 04:26:33

Re: [Solved] C error handling

#15 2011-10-04 06:46:35

Re: [Solved] C error handling

Board footer