Re: [PATCH net-next v9 23/23] testing/selftest: add test tool and scripts for ovpn module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/17/24 05:27, Antonio Quartulli wrote:
On 16/10/2024 23:14, Shuah Khan wrote:
On 10/15/24 19:03, Antonio Quartulli wrote:
The ovpn-cli tool can be compiled and used as selftest for the ovpn
kernel module.

It implements the netlink API and can thus be integrated in any
script for more automated testing.

Along with the tool, 2 scripts are added that perform basic
functionality tests by means of network namespaces.

The scripts can be performed in sequence by running run.sh

Cc: shuah@xxxxxxxxxx
Cc: linux-kselftest@xxxxxxxxxxxxxxx
Signed-off-by: Antonio Quartulli <antonio@xxxxxxxxxxx>

I almost gave my Reviewed-by when I saw the very long argument parsing
in the main() - please see comment below under main().

Let's simply the logic using getopt() - it is way too long and
complex.

Shuan,

while looking into this I got the feeling that getopt() may not be the right tool for this parser.

The ovpn-cli tool doesn't truly excpect "options" with their arguments on the command line, but it rather takes a "command" followed by command-specific arguments/modifiers. More like the 'ip' tool (from iproute2).

The large if/else block is checking for the specified command.
Moreover commands are *mutually exclusive*.

Converting this logic to getopt() seems quite complicated as I'd need to:
* keep track of the first specified command (which may be in any position)
* prevent other commands to be thrown on the command line
* come up with an option for each command-specific argument (and make sure only those required by the specified command are present)


Thank for looking into it. I would like to make a suggestion to
add a parse() routine and move this logic there instead of making
the main() very long. It will be easier to read the code as well.

Are you sure this is the right path to follow?

The 'ip' tool also implements something similar after all.


Sometimes argument parsing takes on life as new options get
added. It starts out as a couple if else conditionals and
expands - when I see a long argument parsing code, I like
to pause and ask the question. Sounds like you case is more
complex.

thanks,
-- Shuah




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux