[Swan-dev] code for impossible situations

D. Hugh Redelmeier hugh at mimosa.com
Mon Aug 31 14:31:17 UTC 2020


If a case is impossible, but you feel unsure about it, use passert.  Then 
you'll know.

Actually handling an impossible case requires or suggests that all related 
code should handle it too.  And that multiplies complexity and the chance 
for bugs.

This code will never be exercised by our test suite.  So that code might 
well have bugs.  It will burn cycles and create confusion in code readers.

Handling an impossible case some places and not others raises questions 
and confusion too.

On the other hand, a passert reduces confusion.  (If it fires, the result 
is educational.)

I'm trying to understand dev_exist_check and its callers and I think this 
is an example of the problem

function dev_exists_check takes a "dev_name" string parameter.

- the code handles a null dev_name by treating it a like an unknown 
  device.  With explicit code.

- if the device is unknown (and another condition that should be 
  irrelevant), the code reports errno, on the assumption that the cause 
  for being unknown is a failed call.

- but wait: there was no call in the case of a NULL.  BUG.

- but wait: I have a hunch that dev_name is never NULL.  So this is only a 
  theoretical bug. Evidence: I put a pexpect in, ran the whole test suite, 
  and never once had this pexpect fire.  This is not proof.

The code path for dev_name == NULL
(1) has a bug
(2) is not tested in our test suite
(3) makes the code harder to understand.

I'm going to replace it with a passert and simplify the code.

There are still mysteries.  find_xfrmi_interface explicitly checks for 
NULL before it calls dev_exist_check.  But not all of the code in that 
routine seems to handle the NULL properly.  It looks to me (with less 
evidence) that
(1) this too is a bug
(2) that is not tested in our test suite
(3) makes the code harder to understand.
(4) the combination of the two is magnifies the problem.

Generally: of all the callers of dev_exist_check, which ones make any 
sense with a NULL dev_name?

(Please don't fix this.  I've got the code on the operating table here.  
I'm painfully swimming upstream.)

Null references: Tony Hoare's Billion Dollar Mistake.
<https://www.youtube.com/watch?v=ybrQvs4x0Ps>



More information about the Swan-dev mailing list