Hacker News Reader

Comment by quotemstr | Hacker News Reader

Counterexample: pthread_mutex_unlock. That function returns an error code, but it cannot possibly fail in a well-formed program. Checking for an error for mutex unlock is pointless: what would you do in response?

View on HN

In the late 90s I had an app ported to multiple unixes. We were having intermittent problems with our HP/UX port which was caused by a mutex being unlocked by a different thread than which had locked it. In that case (which only happened under heavy worker contention) unlock happily returned an error and the mutex was left locked.

Be careful about "can't possibly fail".

> cannot possibly fail in a well-formed program

I think that's your answer. A mutex error return probably indicates an application bug, such as double unlock. You should probably assert or abort on "can't happen" mutex errors.

Programmers are lazy. If they take the time to document an error return value, then you should probably heed their warnings. :)

Indeed. I like using a VERIFY macro:

    #ifdef NDEBUG
    # define VERIFY(x) ((x), 1)
    #else
    # define VERIFY(x) assert((x))
    #endif

Then you can write

    VERIFY(pthread_mutex_unlock(&lock) == 0);

You don't need, however, to consider the possibility of your program continuing to run after pthread_mutex_unlock fails.

man 3 assert

Huh? You do realize that the standard assert() macro already is compiled out if NDEBUG is defined, right?

Your code could just as well be written as

    assert(pthread_mutex_unlock(&lock) == 0);

which of course has the added benefit of not inventing anything new, i.e. being standard and immediately understood by anyone who knows the language and its libraries reasonably well.

Be very careful with that. When compiled under NDEBUG and the assert gone, the mutex unlock will be gone too and you'll wonder why the application stops working.

As per jacquesm's comment below[0], your example would, by default, be compiled out if NDEBUG is defined. So in production you'd never release the lock.

[0] https://news.ycombinator.com/item?id=8206258

You're misreading the code. When NDEBUG is defined, we don't use assert, but instead always evaluate the expression. The overall effect is that we always evaluate VERIFY's argument, but only check it in debug builds.

Replying to self since I can't edit: d'oh. Yes, I totally mis-read the original code. I should have relized why the other comment questioning this practice had been down-voted, heh.

Of course not unlocking the mutex in non-debug builds would be a problem.

Thanks, and sorry.

Never ever put actual code in asserts, not even through clever macros. Stick the return value in a temp var, then check the contents of the temp var in your assertion.

If you ever think "this can't possibly happen" then you should go right ahead and add an assertion to that effect.

When "cannot happen" happens:

1. stop everything

2. coredump (if applicable)

3. return -1 from main

Reply on news.ycombinator.com