[Openvpn-devel] Unified success messages for setting mtu

Message ID 20200630095443.7188-1-cschenk@mail.uni-paderborn.de
State Accepted
Headers show
Series [Openvpn-devel] Unified success messages for setting mtu | expand

Commit Message

Christopher Schenk June 29, 2020, 11:54 p.m. UTC
that makes sense. I updated the patch.

Christopher
---
 src/openvpn/tun.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Gert Doering July 6, 2020, 5:43 a.m. UTC | #1
Acked-by: Gert Doering <gert@greenie.muc.de>

Thanks :-) - given that this is somewhat trivial, I have not actually
run a binary to look at the messages.  I have counted arguments and done
a test build to see if new warnings show up (no).

I *do* see an old warning...

tun.c: In function 'windows_set_mtu':
tun.c:5523:21: warning: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'DWORD {aka long unsigned int}' [-Wformat=]
         msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u if_index=%d]",

mmmh...

Your patch has been applied to the master branch.

commit efe01d52e36c597484b6fa24c4820b6345d08ae6
Author: Christopher Schenk
Date:   Tue Jun 30 11:54:44 2020 +0200

     Unified success messages for setting mtu

     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20200630095443.7188-1-cschenk@mail.uni-paderborn.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20171.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Jonathan K. Bullard July 6, 2020, 6:43 a.m. UTC | #2
Hi,

On Mon, Jul 6, 2020 at 11:43 AM Gert Doering <gert@greenie.muc.de> wrote:
>
> Acked-by: Gert Doering <gert@greenie.muc.de>
>
> Thanks :-) - given that this is somewhat trivial, I have not actually
> run a binary to look at the messages.  I have counted arguments and done
> a test build to see if new warnings show up (no).
>
> I *do* see an old warning...
>
> tun.c: In function 'windows_set_mtu':
> tun.c:5523:21: warning: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'DWORD {aka long unsigned int}' [-Wformat=]
>          msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u if_index=%d]",

It's ok if this is only for Windows code (I assume it is because of
the name of the function) because x86 and ARM have the same
endian-ness and are the only Windows platforms (that I know of).
However, it would still be better to use "%lu" to avoid the warning
and to future-proof it for any _new_ Windows platform that's
different. (Although I admit Microsoft is unlikely to do that.)

Best regards,

Jon

Patch

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 5567c445..2a2df27f 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -5525,7 +5525,7 @@  windows_set_mtu(const int iface_index, const short family,
     }
     else
     {
-        msg(M_INFO, "Successfully set %s mtu on interface %d", family_name, iface_index);
+        msg(M_INFO, "%s MTU set to %d on interface %d using SetIpInterfaceEntry()", family_name, mtu, iface_index);
     }
 }