[Swan-dev] xauth_send_request has a comment that confuses me

[Paul Wouters]

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

> At the end of xauth_send_request:
>
>        /* RETRANSMIT if Main, SA_REPLACE if Aggressive */
> 	/* ??? the actual code seems to force EVENT_v1_RETRANSMIT */
> 	if (st->st_event->ev_type != EVENT_v1_RETRANSMIT) {
> 		delete_event(st);
>                event_schedule_ms(EVENT_v1_RETRANSMIT,
>                  		st->st_connection->r_interval, st);
>        }
>
> I don't understand how the comment matches the code.  Is the comment
> correct?  Is the code?  If both are correct, I suggest the comment
> needs expansion to make this clear.

The comments go back to the initial import of openswan 2.0.0.

I don't understand the comment and I'm happy the code doesn't follow it.

I've removed the comments.

> Bonus issue:
>
> - The same comment and code appear in modecfg_send_request.
>
> - The same comment and almost the same code appear in
>  modecfg_send_set.
>
> Should this be turned into a function?

It's 3 lines, so I'm not worried. But I don't mind either.

> Is the difference in the code in modecfg_send_set important?

Are talking about the order of record_and_send_ike_msg() and
the event rescheduling? Or the fact that it does not check
the existing event and just always replaces it?

The first cause has a comment clarifying it:

         /* Schedule retransmit before sending, to avoid race with master thread */

I don't understand the comment, because I'm pretty sure this is the main thread?
The comment comes from openswan 2.1.4 when xauth was added.

Maybe Antony can tell why we check for EVENT_v1_RETRANSMIT and
EVENT_NULL before setting it to EVENT_v1_RETRANSMIT? My guess is
that checking for EVENT_v1_RETRANSMIT just ensures the old timer
remains, but I'm unsure why. I also don't understand why not
having an event would be reason to still not schedule one?

Paul


> _______________________________________________
> Swan-dev mailing list
> Swan-dev at lists.libreswan.org
> https://lists.libreswan.org/mailman/listinfo/swan-dev
>