Re: [Openvpn-devel] [PATCH] wintun: refactor code to use enum driver type

[Gert Doering]

Hi,

On Thu, Jan 16, 2020 at 03:19:00PM +0100, Simon Rozman wrote:
>  {
> @@ -3676,14 +3692,16 @@ get_tap_reg(struct gc_arena *gc)
>  
>  if (status == ERROR_SUCCESS && data_type == REG_SZ)
>  {
> -if (!strcmp(component_id, TAP_WIN_COMPONENT_ID) ||
> -!strcmp(component_id, "root\\" TAP_WIN_COMPONENT_ID) 
> ||
> -!strcmp(component_id, WINTUN_COMPONENT_ID))
> +enum windows_driver_type windows_driver = 
> WINDOWS_DRIVER_UNSPECIFIED;
> +
> +if ((windows_driver = WINDOWS_DRIVER_TAP_WINDOWS6, 
> !strcmp(component_id, TAP_WIN_COMPONENT_ID))
> +|| (windows_driver = WINDOWS_DRIVER_TAP_WINDOWS6, 
> !strcmp(component_id, "root\\" TAP_WIN_COMPONENT_ID))
> +|| (windows_driver = WINDOWS_DRIVER_WINTUN, 
> !strcmp(component_id, WINTUN_COMPONENT_ID)))
>  {

I could propably figure out what this if() statement with three intentional
assignments in it does, but I'm fairly sure I do not *want* to spend the
time, and I think nobody else should.

Lev has ACKed the patch, and it's WIN32 only - but I would very much
prefer are more readable variant of this.  Like

if ( strcmp(component_id, TAP_WIN_COMPONENT_ID) == 0 ||
 strcmp(component_id, "root\\" TAP_WIN_COMPONENT_ID) )
{
windows_driver = WINDOWS_DRIVER_TAP_WINDOWS6;
}
else if ( strcmp(component_id, WINTUN_COMPONENT_ID) == 0 )
{
windows_driver = WINDOWS_DRIVER_WINTUN:
}

if ( windows_driver != WINDOWS_DRIVER_UNSPECIFIED )
{
... pre-existing code in the if() clause


not many more lines, but way easier to read than on-the-fly assignments
in if(), and also easier to read than the old code.


Pretty please :)

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

Re: [Openvpn-devel] [PATCH] wintun: refactor code to use enum driver type

[Lev Stipakov]

Hi,

While we've discussed this during hackathon, it would
nevertheless be nice to explain in commit message
why this is needed (something like make code more device type agnostic).

Also, this line

  +msg(M_FATAL, "Adapter '%s' is using %s driver, %s expected.
If you want to use this device, adjust --windows-driver.", dev_node,
print_windows_driver(windows_driver),
print_windows_driver(tt->windows_driver));

is a bit too long, it would be nice to split it.

Other than that, looks good.

Built and tested with MSVC.

Acked-by: Lev Stipakov 

-- 
-Lev
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel