Redundant epoch check in dtls_get_bitmap?

[fwh-dc]

Doesn't the check rl->unprocessed_rcds.epoch != rl->epoch always evaluate true?

openssl/ssl/record/methods/dtls_meth.c

Lines 92 to 100 in 925118e

	/*
	* We can only handle messages from the next epoch if we have already
	* processed all of the unprocessed records from the previous epoch
	*/
	else if (rr->epoch == (unsigned long)(rl->epoch + 1)
	&& rl->unprocessed_rcds.epoch != rl->epoch) {
	*is_next_epoch = 1;
	return &rl->next_bitmap;
	}

From the comment it seems more correct to check pqueue_size(rl->unprocessed_rcds.q) == 0?

[mattcaswell]

I think this check (rl->unprocessed_rcds.epoch != rl->epoch) can actually be removed completely.

Things work differently since I refactored the record layer (implemented in 3.2). Prior to that there was one queue of unprocessed records. If you got a record destined for the next epoch you would queue it. When swapping epoch's the first thing you would do is empty the unprocessed records that we'd previously queued and reinject them into the new "current" epoch. In that context this comment makes perfect sense - basically we shouldn't queue new unprocessed records if we haven't finished transferring all the old ones into the current epoch.

After the record layer refactor each epoch has a different record layer object - and each record layer object has its own unprocessed records queue. There will never be records in this queue that are "leftover" from the previous epoch and haven't been transferred yet.

[fwh-dc]

Ok great, I just wanted to make sure I didn't miss something. I'll add a refactoring proposal then.