[Swan] awkward code in ikev1.c

[D. Hugh Redelmeier]

I was just reading programs/pluto/ikev1.c and came across some awkward 
code.  I don't feel safe just checking in these changes so I'll post them 
here.

The original logic is somewhat contorted.  The switch can only go one
way!  The complex if can be absorbed into the switch.  Perhaps there
is an intent that isn't actually captured by the code.

Anyone know a good reason for this?

I think that we can simplify the access to the message ID to:
	md->hdr.isa_msgid
That would eliminate the need for the last if.  If I'm wrong and
st->msgid isn't the same as md->hdr.isa_msgid when st!=NULL, it would
still be a good thing to use when st==NULL.

I think that the st_msgid and isa_msgid fields are in network order.  To 
print them with %08x, you need to wrap them in ntohl().

In the good old days (pre C99 standard), int could be 16 bits. So we
used unsigned long as a safe container for 32-bit quantities.  Current
C standards require int to be at least 32 bits.  The old code is safe.
I'd prefer we still wrote the code that way.  (Why do you think ntohl
ends in l?)

==== Original:

	    if(p->payload.notification.isan_type != R_U_THERE
	       && p->payload.notification.isan_type != R_U_THERE_ACK
		&& p->payload.notification.isan_type != ISAKMP_N_CISCO_LOAD_BALANCE
	       && p->payload.notification.isan_type != PAYLOAD_MALFORMED) {
		
		switch(p->payload.notification.isan_type) {
		case INVALID_MESSAGE_ID:
		default:
		    if (st!= NULL) {
		    	loglog(RC_LOG_SERIOUS
			   , "ignoring informational payload, type %s msgid=%08x"
			   , enum_show(&ipsec_notification_names
				       , p->payload.notification.isan_type), st->st_msgid);
		    } 
		    else {
		    	loglog(RC_LOG_SERIOUS
			   , "ignoring informational payload, type %s on st==NULL (deleted?)"
			   , enum_show(&ipsec_notification_names
				       , p->payload.notification.isan_type));

		    }
		}
#ifdef DEBUG
		if(st!=NULL
		   && st->st_connection->extra_debugging & IMPAIR_DIE_ONINFO) {
		    loglog(RC_LOG_SERIOUS, "received and failed on unknown informational message");
		    complete_v1_state_transition(mdp, STF_FATAL);
		    return;
		}
#endif	    
	    }

==== Revised:
	    switch (p->payload.notification.isan_type) {
	    case R_U_THERE:
	    case R_U_THERE_ACK:
	    case ISAKMP_N_CISCO_LOAD_BALANCE:
	    case PAYLOAD_MALFORMED:
		/* silently ignore these payloads ??? */
		break;

	    case INVALID_MESSAGE_ID:	/* why make this case explicit??? */
	    default:
		loglog(RC_LOG_SERIOUS
		    , "ignoring informational payload, type %s msgid=%08lx"
		    , enum_show(&ipsec_notification_names, p->payload.notification.isan_type)
		    , (long unsigned)ntohl(md->hdr.isa_msgid));
		}
#ifdef DEBUG
		if (st!=NULL && st->st_connection->extra_debugging & IMPAIR_DIE_ONINFO) {
		    loglog(RC_LOG_SERIOUS, "received and failed on unknown informational message");
		    complete_v1_state_transition(mdp, STF_FATAL);
		    return;
		}
#endif	    
	    }