Hi, On Thu, Jun 25, 2020 at 11:37:45AM +0300, Lev Stipakov wrote: > @@ -315,7 +315,7 @@ show_available_ciphers(void) > } > } > > - qsort(cipher_list, num_ciphers, sizeof(*cipher_list), cipher_name_cmp); > + qsort((void *)cipher_list, num_ciphers, sizeof(*cipher_list), > cipher_name_cmp);
I find this one questionable. Qsort wants a void *, but every pointer type
should be convertible to void *, without a warning?
(Adding casts for things that are explicitly always allowed and well-defined
is something I do not like very much - it just makes "more code to read")
> diff --git a/src/openvpn/init.c b/src/openvpn/init.c
> index 2c8db68d..107bb4c9 100644
> --- a/src/openvpn/init.c
> +++ b/src/openvpn/init.c
> @@ -1812,7 +1812,7 @@ do_open_tun(struct context *c)
> #ifdef _WIN32
> /* store (hide) interactive service handle in tuntap_options */
> c->c1.tuntap->options.msg_channel = c->options.msg_channel;
> - msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned int)
> c->options.msg_channel);
> + msg(D_ROUTE, "interactive service msg_channel=%u", (unsigned long long)
> c->options.msg_channel);
This gets a NAK. "%u" is "unsigned int", not (never!) (unsigned long long) -
if you want that, because the data type is so big, use %llu or %I64u
(Windows special, according to common.h).
> index 04868cd6..8178ff06 100644
> --- a/src/openvpn/mtu.c
> +++ b/src/openvpn/mtu.c
> @@ -175,7 +175,7 @@ set_mtu_discover_type(int sd, int mtu_type, sa_family_t
> proto_af)
> #if defined(HAVE_SETSOCKOPT) && defined(IP_MTU_DISCOVER)
> case AF_INET:
> if (setsockopt
> - (sd, IPPROTO_IP, IP_MTU_DISCOVER, &mtu_type,
> sizeof(mtu_type)))
> + (sd, IPPROTO_IP, IP_MTU_DISCOVER, (const char
> *)&mtu_type, sizeof(mtu_type)))
I find this questionable as well. Setsockopt() on Unix wants a
"void *", not a "const char *" - and this cast suggest to the reader
that we're dealing with "something character based", which it is not.
> #if defined(HAVE_SETSOCKOPT) && defined(IPV6_MTU_DISCOVER)
> case AF_INET6:
> if (setsockopt
> - (sd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, &mtu_type,
> sizeof(mtu_type)))
> + (sd, IPPROTO_IPV6, IPV6_MTU_DISCOVER, (const char
> *)&mtu_type, sizeof(mtu_type)))
Same here.
> show_available_tls_ciphers_list(const char *cipher_list,
> const char *tls_cert_profile,
> - bool tls13);
> + const bool tls13);
I'm not sure why this is needed or beneficial? This const stuff is
good for pointers ("do not modify the pointed-to area") but for
call-by-value arguments, the compiler can figure this out itself.
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -233,10 +233,11 @@ do_set_mtu_service(const struct tuntap *tt, const short
> family, const int mtu)
> sizeof(set_mtu_message_t),
> 0
> },
> - .iface = {.index = tt->adapter_index,.name = tt->actual_name },
> + .iface = {.index = tt->adapter_index},
Sure of that? As in "are you sure the service does not need the name,
never"?
> .mtu = mtu,
> .family = family
> };
> + strncpynt(mtu_msg.iface.name, tt->actual_name,
> sizeof(mtu_msg.iface.name));
Well spotted. This is interesting, the assignment ".name = tt->actual_name"
shouldn't be legal (as it's basically a hidden strcpy(), not a integral
type or structure assignment), but mingw is not complaining and the
resulting binary works. But this definitely is a necessary bugfix.
> if (!send_msg_iservice(pipe, &mtu_msg, sizeof(mtu_msg), &ack, "Set_mtu"))
> {
> @@ -889,7 +890,7 @@ add_route_connected_v6_net(struct tuntap *tt,
> }
>
> void
> -delete_route_connected_v6_net(struct tuntap *tt,
> +delete_route_connected_v6_net(const struct tuntap *tt,
> const struct env_set *es)
I might be convinced that this is a useful change, but if it's needed(!)
for delete_route_connected_v6_net(), the "cost" should be added to
add_route_connected_v6_net() as well.
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
