2

As everybody knows, C allows us to write anything we want. There comes one big problem — we are the only who responsible to handle errors that comes from standard library functions and, therefore, write reliable code.

The goal is to make writing reliable code easier.

And the question is about errors that comes from standard library functuins.

I have an idea to wrap standard functions so that we can’t hide errors. If we want to ignore errors, we’ll need to explicitly do that.

This code shows my solution:

void
safe_access
        (bool *ok, const char *path, int amode)
{
        // reset error
        *ok = true;
        errno = 0;

        int ret = access(path, amode);

        // indicate error
        if (ret) {
                *ok = false;
        }
}

This functuon wraps access(3) from unistd.h. It solves the followind three problems:

  1. It resets errno, so we won’t be able to check for old errno:
// this function sets errno
access(...);

// we forget to reset errno and everything seems to be OK
// now, we want to check for errno
// but, unfortunately, we’re prone to check old errno that comes from access()
long a = strtol(...);
if (... && errno) {
}
  1. It disallows us to forget handling errors. Even when we want to ignore errors, we need to explicitly do that:
bool ok;
safe_access(&ok, ...);

// we need something to do with `ok`

Also, we can tune warnings to disallow us to keep variables unused.

  1. Everybody will check errors the same way. This is not regular but sometimes we need to check errors differently than just if-not-zero, if-negative, or if-NULL. For example, we need to use ferror(3) after fread(3). My solution is to write safe_fread() that will do it.

Are there any risks and what are they?

  • 2
    Ok. How do you plan on enforcing the use of the replacement functions? Who watches the watchers? – Robert Harvey May 7 at 16:05
  • Wrap your C code in C++, and throw Exceptions for error conditions, or just code as pure C++ with the throwing of Exceptions for error conditions. – mrflash818 May 7 at 16:06
  • 2
    Standard library functions (malloc(3), fopen(3), access(3), stat(3), etc) set errno to indicate errors. The scope is global, @candied_orange – David Lehnsherr May 7 at 16:14
  • 1
    I want to code in pure C, @mrflash818 – David Lehnsherr May 7 at 16:15
  • 1
    Generally, one uses the return-value to determine whether an error occurred, though errno is sometimes needed for disambiguation. It is only cleared if (potentially) needed for disambiguation or manually, and set if an error occurred. – Deduplicator May 7 at 21:38
11

Adding a false impression of security is a very bad idea.

It will not remove bad habits

If your colleagues today are not disciplined enough to handle calls with appropriate error checking :

if (!access ("input.txt", R_OK) || !access ("output.txt", W_OK)) {
    // something went wrong, so I have to check fro errno
    perror ("Required file not accessible"); 
} 

They will as well continue to ignore errors by just not processing your boolean:

bool ok; 
safe_access (&ok,"input.txt", R_OK); 
safe_access (&ok,"output.txt", W_OK);
// oops forgot to check the first one 

But it will break usual idioms

Since you change the signature of the function in a rather radical way, a couple of well known idioms will no longer work.

I think for example of combining several error checks in an || chain to stop at first error. But even more problematic, usual loops :

while (fgets(buffer, BUFLEN, fpin)) {
    // do something 
}

Do you really want to use:

safe_fgets (&ok, buffer, BUFLEN, fpin); 
while (ok) {
    // do something 
    safe_fgets (&ok, buffer, BUFLEN, fpin);  // DRY !!!
}

And it might introduce new risks

From the previous example, you could conclude that your safe_xxx() functions should at least use the same return type than the original function, which would be less disruptive.

But then you may face undefined behavior if in an expression you'd combine several safe_xxx() calls with the same &ok and if these calls would be indeterminately-sequenced.

Not to speak of parameter mismatch or passing by error a NULL pointer.

Finally, in the additional risks I would also mention that you might create legitimate expectations. For instance, I'd expect the following call to be safe, since the function advertise safety:

char *path=NULL; 
// do something 
safe_access (&ok, path, R_OK);  

So in the end, seeing safe everywhere could make me less prudent, assuming that the function would do some check which it doesn't.

Here I'm less sure, but could it be possible that static code analysers might not find some risks, since identifiable risky call is wrapped into a separate library/compilation unit which could defeat constant propagation ? I'm thinking in partiular to buffer overflowing rules

Loss of productvity

In addition, manny errno relevant functions are well known. Forcing to use your alternate one might impact productivity:

  • new team members need to get used to it
  • you will anyway have to check that they respect the behavior
  • colleagues will type longer names (less readable)
  • colleagues will need to refactor knwown idioms
  • code reuse from one project to the other might be less easy
  • third party libraries will anyway not use your approach
  • lots of boilerplate code

There is a better alternative

It's called code review and peer programming: just make sure that people properly check errors

  • nicely explained – Sazzad Hissain Khan May 7 at 18:54
  • Well, “it will not break bad habits” along with “loss of productivity” sound convincing to me. Now I think I added another level that solves 0 problems but takes a lot of energy to write code this way. Thanks. – David Lehnsherr May 7 at 19:47
1

Here are some analysis,

Benefits:

  1. Manual API error checking not needed
  2. Improves resilience when forced to implement

Risks:

  1. Enforcing to use wrapper requires extra monitoring burden
  2. Error scope (ret type) is narrowed down and can't be used for many similar calls
  3. Extra global state variable bool ok increases app state complexity
  4. Only limited for pattern (path, mode) -> error not useful for other
  5. Wrapping always cost some performance
  • I think bool ok is supposed to be local, not global. That's the whole point - you need to create a local variable in order to call the function, so you cant ignore the error. – pschill May 7 at 17:01
  • You’re right, @pschill, thank you for good clarification. – David Lehnsherr May 7 at 17:44
  • This analysis helps me, thank you. – David Lehnsherr May 7 at 17:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.