CODEWITHSUNDEEP
cerror-handling
Wei Shi
Wei Shi

Reputation: 5055

Clean error handling approach in C

I'm writing a C program that needs good error handling. The code likes like this:

If(doWork("A")<0){
    return -1;   
}
If(doWork("B")<0){
    undoWork("A");
    return -1;
}
If(doWork("C")<0){
    undoWork("A");
    undoWork("B");
    return -1;
}
return 0;

This code works but looks very messy, especially I have a long list of doWork(X) to call. Is there a better and cleaner approach to handle error in this case?

Upvotes: 7

Views: 1432

Answers (6)

Isso
Isso

Reputation: 1333

There is also another widely used approach based on a single pass loop that is clear and doesn't require goto. It implies though that Undo functions correctly handle both work that was done and that was not.

do
{
  if(doWork("A")<0)
    break;   

  if(doWork("B")<0)
    break;

  if(doWork("C")<0)
    break;

  return 0;
}
while(0);

undoWork("A");
undoWork("B");
undoWork("C");
return -1;

Upvotes: 0

Flavius
Flavius

Reputation: 13826

Being the same task doWork, you can probably define a linked list or vector of jobs and pass that as a parameter to doWork, append the corresponding information to this list inside the function, and only call undoWork once:

If(doWork("A", &jobs)<0){
    return -1;   
}
If(doWork("B", &jobs)<0){
    undoWork(jobs);
    return -1;
}
If(doWork("C", &jobs)<0){
    undoWork(jobs);
    return -1;
}
return 0;

This way, your logic will not become overly complicated, no matter the combination of jobs to be undone.

The advantage, compared to @twain249's solution, is that the function decides whether a job is added to the list or not, so you've got a nice isolation, modularity.

You can of course combine some form of an interable data structure with this, to further reduce the amount of repetitive code:

for(i=0; i < jobdata.size; i++) {
    If(doWork(jobdata[i], &jobs)<0){
        undowork(jobs);
        return -1;   
    }
}

As you can notice, data structure design plays an important role in algorithm design, usually a much more important one than one usually thinks.

There could be thousands of jobs, the code will remain a four-liner.

Upvotes: 2

pizza
pizza

Reputation: 7640

If you don't have a super long list, you can approach it this way.

if (dowork("A") >=0) {
if (dowork("B") >=0) {
if (dowork("C") >=0) {
if (dowork("D") >=0) return 0;
undowork("C"); }
undowork("B"); }
undowork("A"); }
return -1;

Upvotes: 0

George Skoptsov
George Skoptsov

Reputation: 3971

Some people, especially beginner-to-intermediate programmers, have a very idiosyncratic reaction to seeing goto in production code, but the usual idiom for sequential acquiring of resources and their intelligent release upon error is the following:

if(doWork("A") < 0)
  goto errA;

if(doWork("B") < 0)
  goto errB;

if(doWork("C") < 0)
  goto errC;

/* success! */
return 0;

/* Error handling / releasing resources section */
errC:
  undoWork("B");
errB:
  undoWork("A");
errA:

return -1;

You will see plenty of examples in system code, e.g. in the linux kernel.

Upvotes: 7

twain249
twain249

Reputation: 5706

if it's possible to store all things you have to call doWork on in an array then you could shorten the code significantly something like.

int i = 0;
int len = MAX_NUM; //set to the value of calls
int error = 0;

for(i = 0; i < len; i++) {
    if(doWork(a[i]) < 0) {
        error = 1;
        break;
    }
}

if(error) {
    for(int j = 0; j < i; i++) {
        undoWork(a[j]);
    }
    return -1;
}

Upvotes: 0

Jonathan Wood
Jonathan Wood

Reputation: 67345

Probably not. Newer languages like C++ and C# favor exceptions to help improve situations just like this.

Perhaps you could have a table that somehow indicated which tasks you've done and undo those. But I really think that would make your code more complex and not less.

Also note that, while there are some pretty strong feelings about using goto, there are in fact times when that can simplify structures like this.

Upvotes: 0

Related Questions