On Wed, May 10, 2023 at 01:57:00PM +0200, Gert Doering wrote: > Hi, > > On Wed, May 10, 2023 at 01:22:35PM +0200, Frank Lichtenheld wrote: > > } > > - mssval = (opt[2]<<8)+opt[3]; > > + mssval = opt[2] << 8; > > + mssval += opt[3]; > > Is this an intentional change, or just a side effect of "something > intermediate"?
This fixes a conversion warning: ../../../src/openvpn/mss.c:196:26: warning: conversion from ‘int’ to ‘uint16_t’ {aka ‘short unsigned int’} may change value [-Wconversion] 196 | mssval = (opt[2] << 8) + opt[3]; | ^ So basically the compiler goes up to int and then realises it needs to go back down to uint16_t. By splitting the statement this is avoided. One could also fix that with casts, but that would be much uglier, IMHO. > > > @@ -7260,6 +7260,7 @@ add_option(struct options *options, > > /* value specified, assume encapsulation is not > > * included unless "mtu" follows later */ > > options->ce.mssfix = positive_atoi(p[1]); > > + ASSERT(options->ce.mssfix <= UINT16_MAX); > > options->ce.mssfix_encap = false; > > options->ce.mssfix_default = false; > > This part of the patch is making me unhappy, thus, NAK. We do have > a way to signal option errors, and ASSERT() is not... your code > would make a client ASSERT() if a server pushes "mssfix 70000". Okay, will change. Regards, -- Frank Lichtenheld _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel