[L] Change in openvpn[master]: Add lookup of multi session by session id
"flichtenheld \(Code Review\) via Openvpn-devel" <[email protected]>
| Newsgroups | gmane.network.openvpn.devel |
|---|---|
| Message-ID | <6b4618739f3cb09b1211ec06646c238950d46563-EmailReviewComments-HTML@gerrit.openvpn.net> |
Attention is currently required from: plaisthos.
flichtenheld has posted comments on this change by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1727?usp=email )
Change subject: Add lookup of multi session by session id
......................................................................
Patch Set 3: Code-Review-2
(10 comments)
File doc/man-sections/advanced-options.rst:
http://gerrit.openvpn.net/c/openvpn/+/1727/comment/063287a6_ad77a9f8?usp=email :
PS3, Line 32: address table to ``v``. And if specified the session id hash table to
"If specified set the size of the session id hash table to"
http://gerrit.openvpn.net/c/openvpn/+/1727/comment/adc8d07e_8b554a73?usp=email :
PS3, Line 33: ``s``. Otherwise ``s`` be set to the same value as ``r``.
"will be"
File src/openvpn/mudp.c:
http://gerrit.openvpn.net/c/openvpn/+/1727/comment/d3040fdc_2606f6b9?usp=email :
PS3, Line 383: if (m->top.c2.buf.len < 9)
Why hardcode 9 instead of "sizeof(struct session_id) + 1" ?
File src/openvpn/multi.h:
http://gerrit.openvpn.net/c/openvpn/+/1727/comment/f186bf4c_f7a8e8df?usp=email :
PS3, Line 138: * id here is non-null and the hash map's key pointer uses is pointing
Not sure what "uses" is supposed to mean here?
http://gerrit.openvpn.net/c/openvpn/+/1727/comment/4df36e1e_1916a232?usp=email :
PS3, Line 139: * here */
What is "here"? Sounds like it points to this part of the struct, but it actually points to the beginning of the struct, doesn't it?
http://gerrit.openvpn.net/c/openvpn/+/1727/comment/2fd8e60c_76963cb3?usp=email :
PS3, Line 178: session id. We do not care collisions here
"care about"
File src/openvpn/multi.c:
http://gerrit.openvpn.net/c/openvpn/+/1727/comment/ad114933_bf7dfcce?usp=email :
PS3, Line 441: if (mi->context.c2.tls_multi && true)
"&& true" ?
File src/openvpn/sid_hash.h:
http://gerrit.openvpn.net/c/openvpn/+/1727/comment/30682dc5_7bd0156f?usp=email :
PS3, Line 35: /* This only be called if the multi instance is not already present
"This should" or "This must"?
http://gerrit.openvpn.net/c/openvpn/+/1727/comment/1001dcdb_16f4167a?usp=email :
PS3, Line 52: struct hash_element *he = hash_lookup_fast(m->sid_hash, bucket, sid, hv);
Could reduce code duplication slightly by using multi_hash_sid_lookup here.
File tests/unit_tests/openvpn/test_misc.c:
http://gerrit.openvpn.net/c/openvpn/+/1727/comment/c82c40ee_2c2abdc4?usp=email :
PS3, Line 143: get_random(void)
```
test_misc.c: In function ‘test_list’:
test_misc.c:258:19: error: conversion from ‘long int’ to ‘uint32_t’ {aka ‘unsigned int’} may change value [-Werror=conversion]
258 | inc = (get_random() % 3) + 1;
| ^
```
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1727?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: Idb59ecd119331b198792ad1379bec8600211651b
Gerrit-Change-Number: 1727
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Comment-Date: Mon, 06 Jul 2026 12:47:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel