[Swan-dev] // considered a warning sign

[Paul Wouters]

On Sun, 1 Oct 2017, D. Hugh Redelmeier wrote:

> I'm exploring the use of // in libreswan code.  It seems to get used for
> temporary edits, some of which are scary.

Yes. Our coding style doesn't allow it, so people tend to use it for
temp markers to find things back while working. Usually not meant to
last past development.

> I just fixed one with 07b03441a448079b23915d8ed2cb4d8775abd10d.  I
> don't actually know if the case would ever be executed.  If not, the
> original code (a bad_case call) would have been correct.  If so, the
> code would have gone into the weeds.

Thanks for the fix. Although I changed the return NULL to return FALSE.

> I'm now looking at a // in ikev2_send_auth.  Seems to turn a failure
> into an incorrect success.  Why is that a good idea?
> programs/pluto/ikev2_parent.c:2801:			//return STF_FAIL + v2N_NO_PROPOSAL_CHOSEN;

Fixed.

> // is used to describe fields in struct kernel_sa which are apparently
> needed:
> programs/pluto/kernel.h:135:	// uint32_t sa_priority;
> programs/pluto/kernel.h:136:	// struct sa_marks *sa_marks;

Fixed to use /* */

> Other suspicious uses:
>
> programs/pluto/kernel_netlink.c:727:  // req.u.p.lft.soft_use_expires_seconds = deltasecs(use_lifetime);
>
> programs/pluto/kernel_pfkey.c:1252:		//*allocations = sal->sadb_lifetime_allocations;
> programs/pluto/kernel_pfkey.c:1255:		//*use_time = sal->sadb_lifetime_usetime;
> programs/pluto/kernel_pfkey.c:1256:		//*packets = sal->sadb_x_lifetime_packets;

These were left as "documentation" as well.

> programs/pluto/myid.c:53:	// char tmpid[IDTOA_BUF];
> programs/pluto/myid.c:55:	// idtoa(id, tmpid, sizeof(tmpid));
> programs/pluto/myid.c:56:	// loglog(RC_LOG_SERIOUS,"resolve_myid() called for id:%s",tmpid);

this was a temp edit. We found that resolve_myid() was called in atoi()
which is called a lot, and the idtoa() caused a DNS lookup, so this was
quite some overhead for a logline. I had commented it out just to see
the effects. I shold have properly deleted it, which I did now.

Paul