Hi, On Tue, Jul 19, 2022 at 04:16:39PM +0200, Antonio Quartulli wrote: > open_tun_generic already contains the logic required to find a device > name when not specified b the user. For this reason the DCO case can > easily leverage on function and avoid code duplication. > > Signed-off-by: Antonio Quartulli <a...@unstable.cc>
*sigh* First of all, my apologies for letting you jump through all these hoops - I'm NOT doing this to annoy you or drag my feet, but really to keep the code readable and maintainable. That said, I've spent two days now pondering the latest version of this patch, and I can not ACK it - it's ugly beyond openvpn standards. My main gripe is that we have this open_tun_generic() function which is used for "all but linux (and Windows)". So you add a call to it now for the DCO call (this is okay-ish) but then it's not actually *using* the code paths in open_tun_generic(), but adding new and linux-exclusive code paths #ifdef TARGET_LINUX. So, on Linux with DCO, very little of that function is actually used (half of it is #ifdef TARGET_LINUX, and the non-#ifdef parts are "else" branches for "not DCO" - but "for not DCO", we do not call this in the first place) - all that remains is a for() loop to iterate device names, and a string_alloc() call... That said, I've cooked up a v11 of 05, which introduces a much leaner open_tun_dco_generic() function - to be shared by Linux and FreeBSD - which does "only DCO things", leaving open_tun_generic() alone. I've tested this with the usual test cases --dev tap (works, because options.c will already disable DCO) --dev tun (iterates to "tun7" on my DCO test system) --mktun --dev tun471 ; use --dev tun471 (works, falls back to tun) --dev tun472 (works as DCO device)) Right now this is sitting on top of my clone of your "dco" tree - I'll see that I can apply this to the right parts in the middle and send a 05 v11 out later, then we can haggle on the least ugly way forward. gert PS1: I'm fairly sure *I* did the existing #ifdef TARGET_NETBSD in open_tun_generic() and I should not have done that. Bad precedence. PS2: 05 v10 works, and does not break anything, But It Is Too Ugly. -- "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