[Openvpn-devel] [L] Change in openvpn[master]: PUSH_UPDATE: Added update_option() function.

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, mrbff, plaisthos.

cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/810?usp=email )

Change subject: PUSH_UPDATE: Added update_option() function.
......................................................................


Patch Set 21: Code-Review-1

(5 comments)

Patchset:

PS21: 
Nagging about code duplication...


File src/openvpn/options.c:

http://gerrit.openvpn.net/c/openvpn/+/810/comment/e80b547e_689dbd50 :
PS21, Line 5897:             }
I find this duplication of syntax checking, mmmh, sub-optimal.  We'll check the syntax anyway in `add_option()` later on...

I could see user complaints about "I sent a PUSH_UPDATE with one invalid `route` in it, and all my routes were lost", but honestly, do we really care that much?  (If yes, we could consider moving the syntax check into a small helper, if not, just drop the lines from `rol_check_alloc()` up to this line).


http://gerrit.openvpn.net/c/openvpn/+/810/comment/e1f5b5bd_f34b1d7f :
PS21, Line 5927:             }
same here


http://gerrit.openvpn.net/c/openvpn/+/810/comment/4054ed13_3dac7037 :
PS21, Line 6009: 
same here... lots of code that needs to be in sync with the add_option() checks...


http://gerrit.openvpn.net/c/openvpn/+/810/comment/abc555fa_e0e5856c :
PS21, Line 6045: #endif
I note a distinct lack of syntax checking for `dhcp-option` ;-)



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

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia45c99e6df7b3ad24020c10b8a9b3577984ecdc2
Gerrit-Change-Number: 810
Gerrit-PatchSet: 21
Gerrit-Owner: mrbff <ma...@ma...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-Reviewer: stipa <lst...@gm...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: mrbff <ma...@ma...>
Gerrit-Comment-Date: Mon, 21 Jul 2025 19:21:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment