[Swan-dev] libswan/addrtot.c: simplify so scan-build can understand it

[D. Hugh Redelmeier]

| From: Andrew Cagney <andrew.cagney at gmail.com>

| On Tue, 19 Feb 2019 at 00:33, D. Hugh Redelmeier
| <hugh at vault.libreswan.fi> wrote:
| >
| > New commits:
| > commit b9cd04d93e96ed04baf3a5d25eb50e1ea2d8370f
| > Author: D. Hugh Redelmeier <hugh at mimosa.com>
| > Date:   Tue Feb 19 00:21:22 2019 -0500
| >
| >     libswan/addrtot.c: simplify so scan-build can understand it
| 
| BTW, what is scan-build?  A search suggests something in LLVM?

(Darn, more hidden lore.  Something I rail against and yet am guilty of.)

scan-build is a very convenient lint-like thing that uses clang (LLVM's C 
compiler).

scan-build is a perl script that somehow gets the job done, but I haven't 
checked how.  It does have a man page.  Kind of an obscure name.

How I use it:

	make clean
	scan-build make base

The first line is superstitious: I want to make sure clang and gcc don't 
meet awkwardly, and I want to make sure all of Libreswan's code get's 
built.  Well, all that can be built on my machine, which doesn't run
libreswan.

How should we capture this lore?  The natural place would be the
makefile, but then we get awfully meta.

| >     - the result is also easier for humans to read too.
| >
| >     - Since 30444640cd234d2e1d1e1f29bd565fda661c7b94 there are two copies
| >       of addrtot.c and this only updates one!  The other is frozen.
| >       Too bad that history will be lost when the other is deleted.
| >
| >     - There is a program to test this code: testing/ipcheck/.  "make base"
| >       compiles the test program into OBJ.*/testing/ipcheck/ipcheck.
| >       Unfortunately, nothing in the test suite runs it.  Run the program
| >       manually and compare the output with (non-existent!) reference
| >       output.
| 
| Umm. "ipcheck" does include reference output (if there's a difference
| it sends it to STDERR, and reports it as a failure count / status
| code).

I saw no evidence that our test suite runs it.

When I manually ran it, it just poured out logging of what looked to be 
correct tests.

I tested by running it without my change and with my change and compared 
the output.  It was identical, so I declared victory.

Where is the reference output?  I didn't see it in libreswan/testing/ipcheck/

BTW: unit testing, like this, is great!  So much faster and clearer.

|  However, it is testing
| str_address_{raw,cooked,sensitive,reversed}() and not addrtot() (to
| your point "the result is also easier for humans to read too" it was
| easier than trying to fix edge cases in addrtot()).

Yikes!  Are you saying that I didn't actually test my changes?

Henry build a bunch of unit tests into the library.  Generally in the
source files themselves.  I guess they've been removed.  No history was
available for this source file.

Good luck to everyone who encounters a bug in this untested change!
I'm not around today.  If bitten, revert.

|  Anyway, like for
| subnettot() - 6a7bcbe5c1823cbc1df70d0a32dd3e5d36992a47 - I suspect
| things are reaching the point where addrtot() can reduced to a
| str_address_raw() wrapper; however not right now.

I have no opinion at the moment (i.e. I haven't researched this).

I was driven by a persistent scan-build warning (that was wrong).  So:
bottom up; not driven by a broad vision.  Broad visions are good.