Share via

Errors and a simple invariant

  • Article

Before addressing the problems with close_resource(), I want to explore another avenue which is invariant restoration.

Let's amend the little function some more.  I'm trying to stay away from Windows concepts but I need to update a global counter in what is presumably a multithreaded context.  Intead of pulling in pthreads and mutexes, I'm going to call some functions called interlocked_increment() and interlocked_decrement().  They atomically update an integral type.  [I also in a fit of revisionism changed the parameter from a non-const char * to const char *.  I unfortunately was there during the const poisoning debates and some habits, even when you know they're bad, die hard...]

size_t updates_in_progress = 0;

int do_work(const char *psz) {
resource *pr = NULL;
int t;
interlocked_increment(&updates_in_progress);
if ((t = open_file_as_resource(&pr, psz)) != 0) return t;
if ((t = swap_order_of_bytes_in_resource(pr)) != 0) { close_resource(pr); return t; } // bug??
if ((t = close_resource(pr)) != 0) return t;
interlocked_decrement(&updates_in_progress);
return 0; // success
}

Ok, so that obviously created a new set of bugs.  Again, I'm avoiding syntactic sugar that could make this look a lot easier because my eventual goal is to build a case that code used to either release resources or restore invariants is very special so I want to show all the places it really ends up being used, not just the obvious ones.

Here's the fixed version in C:

size_t updates_in_progress = 0;

int do_work(const char *psz) {
resource *pr = NULL;
int t;
interlocked_increment(&updates_in_progress);
if ((t = open_file_as_resource(&pr, psz)) != 0) {
interlocked_decrement(&updates_in_progress);
return t;
}
if ((t = swap_order_of_bytes_in_resource(pr)) != 0) {
close_resource(pr); // bug??
interlocked_decrement(&updates_in_progress);
      return t;
}
if ((t = close_resource(pr)) != 0) {
interlocked_decrement(&updates_in_progress);
return t;
}
interlocked_decrement(&updates_in_progress);
return 0; // success
}

In some future post after we discuss the reliability contracts of functions like close_resource() and interlocked_decrement() I'll compare and contrast the flavors of syntactic sugar available to make this prettier.

Comments