Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change by flichtenheld. ( 
http://gerrit.openvpn.net/c/openvpn/+/1298?usp=email )

Change subject: ssl: Fix conversion warning in tls_prepend_opcode_v1
......................................................................


Patch Set 3: Code-Review-1

(2 comments)

Patchset:

PS3:
I think the assert is wrong.


File src/openvpn/ssl.c:

http://gerrit.openvpn.net/c/openvpn/+/1298/comment/84eda4a7_8aea915f?usp=email :
PS3, Line 3985:     ASSERT(ks->key_id <= 1 << P_OPCODE_SHIFT);
Can we add brackets here? I find this a bit hard to read since the order of <= 
and << is not so super obvious. Brackets would make this a bit nicer to read.

I think we should probably add a define to indicate the highest key ID, 
something like this:

/* packet opcode (high 5 bits) and key-id (low 3 bits) are combined in one byte 
*/
#define P_KEY_ID_MASK  0x07
#define P_OPCODE_SHIFT 3
/* We use 3 bits for the key id which makes 7 the highest key id */
#define MAXIMUM_KEY_ID 7

I think this assert is also wrong since (1 << 3) = 8 and 8 is not an allowed 
key id.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1298?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I34584f695ddca3b3e1f2bbcb4380ac91b09c1c8d
Gerrit-Change-Number: 1298
Gerrit-PatchSet: 3
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-Comment-Date: Tue, 11 Nov 2025 11:10:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to