gpsd-dev
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[gpsd-dev] Policy on assert(3) use in GPSD


From: Eric S. Raymond
Subject: [gpsd-dev] Policy on assert(3) use in GPSD
Date: Fri, 30 Jan 2015 09:50:42 -0500 (EST)

I think the asserts Gary added to gpsd_acquire_reporting_lock() and
gpsd_release_reporting_lock() were mistaken, and not just because he
got the test backwards.  That slip could happen to anybody, was easily
caught, and doesn't bother me at all. The real problem is more subtle.

gpsd - I'm speaking of the daemon itself, not the clients and test
tools - runs in a lot of contexts where it provides a life-critical
service. We can't allow it to faint every time it gets a case of the
vapors.  Instead, the right thing is to log a problem and soldier on.
If the fault condition is in the logging machinery itself, the right
thing is to just soldier on.

Accordingly, there are very few asserts in the core code.  I count 11,
including Gary's two.  I'm going to remove those two, I've found a third
that can be replaced with error logging, and I'm going to spend the
next couple of hours trying to do that to more of them.

Here's the policy for the daemon and its service libraries.  This
doesn't apply to client code and tools, which can be more relaxed
because the cost of having them go tits-up is lower and recovery
is easier.

1. Use asserts sparingly.

2. One valid use is to pass hints to splint and other analyzers. This
sort of assert should be guarded with #ifdef S_SPLINT_S or #ifdef
__COVERITY_ so we get the benefit of asserting the invariant without
the risk of the assertion failing in production.

3. Another use is to document should-never-happen invariants of the
code.  Write this sort only if you are confident it will never fire
in a production release; it's there to catch when changes in a
*development* revision break an invariant.

4. Outside the above two cases, do not assert when you can log an internal
error and recover.

5. Never use assert() to check resource exhaustion conditions or
other dynamic errors. (This was Gary's mistake.) 

I will add a version of this policy to the Hacker's Guide.
-- 
                <a href="http://www.catb.org/~esr/";>Eric S. Raymond</a>

It is proper to take alarm at the first experiment on our
liberties. We hold this prudent jealousy to be the first duty of
citizens and one of the noblest characteristics of the late
Revolution. The freemen of America did not wait till usurped power had
strengthened itself by exercise and entangled the question in
precedents. They saw all the consequences in the principle, and they
avoided the consequences by denying the principle. We revere this
lesson too much ... to forget it        -- James Madison.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]