Attention is currently required from: flichtenheld, plaisthos.

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

Change subject: Remove openvpn_snprintf and similar functions
......................................................................


Patch Set 2:

(5 comments)

Patchset:

PS2:
some nit picks below..


File src/openvpn/proxy.c:

http://gerrit.openvpn.net/c/openvpn/+/547/comment/d2c89116_22dbc48e :
PS2, Line 962:                 if (sret >= sizeof(buf))
if this can truly happen, does it mean that the buffer is undersized compared 
to the size of all variables we are putting together?

Therefore, wouldn't it make more sense to extend the size of the buffer to 
ensure that no matter what we save in those variables, we will always be able 
to create the HTTP header?

Or there is a limit with the HTTP header that we have to deal with?

My concern is that we are not preventing people from filling those variables as 
they please, but we will then fail to put them together for no good reason.

does it make sense?


File src/openvpn/socks.c:

http://gerrit.openvpn.net/c/openvpn/+/547/comment/8bdf0e3c_8356c931 :
PS2, Line 114:                         (int) strlen(creds.username), 
creds.username,
normally we don't put a paceb etween the cast and the variable name.
This comments applies to all other casts below


http://gerrit.openvpn.net/c/openvpn/+/547/comment/b417986e_a5f333df :
PS2, Line 116:     ASSERT(sret <= sizeof(to_send));
why ASSERT here while in other cases we just go to error or cleanup?


File tests/unit_tests/openvpn/test_buffer.c:

http://gerrit.openvpn.net/c/openvpn/+/547/comment/f8c8a505_dbfb9729 :
PS2, Line 369:      * for this unit test. We know that are doing this that are 
truncated
I think there is some typ0 here.
Maybe something like: "We know that results will be truncated and we actually 
want to test that".



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/547?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: I07096977e3b562bcb5d2c6f11673a4175b8e12ac
Gerrit-Change-Number: 547
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arne-open...@rfc2549.org>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: ordex <a...@unstable.cc>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-Comment-Date: Wed, 27 Mar 2024 10:48:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to