Clean error handling approach in C

Question

I am writing a C program that requires good error handling. I like the code 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 it looks very dirty, especially I have a long doWork(X) list to call. Is there a better and cleaner approach for handling errors in this case?

Answer 1

Some people, especially beginner-to-intermediate programmers, have a very peculiar reaction to viewing goto in production code, but the usual idiom for sequential acquisition of resources and their intellectual release on error is as follows:

 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; 

In the system code you will see many examples, for example. in the linux kernel.

Answer 2

Being the same doWork task, you can probably define a linked list or jobs vector and pass this as a doWork parameter, add 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; 

Thus, your logic will not become overly complex, no matter which combination of tasks is canceled.

The advantage over the @ twain249 solution is that the function decides whether the task is added to the list or not, so you have good isolation, modularity.

Of course, you can combine some form of interdependent data structure with this to further reduce the amount of duplicate code :

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

As you can see, the structure of the data structure plays an important role in the development of algorithms, usually much more important than usually thinks.

There may be thousands of tasks, the code will remain four-line.

Answer 3

Probably not. Newer languages, such as C ++ and C #, prefer exceptions to help improve things this way.

Perhaps you may have a table that somehow indicated which tasks you completed and cancel them. But I really think this will make your code more complex and no less.

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

Answer 4

if it is possible to save all the things you should call doWork in an array, you could greatly reduce the code, for example.

 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; } 

Answer 5

If you don't have a super long list, you can approach it that 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; 

Answer 6

There is also another widely used single pass loop approach that is clear and does not require goto. This implies that the Undo functions correctly handle the work in progress, but this is not the case.

 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;