Re: [PATCH iproute2 v3 1/3] configure: support --param=value style
From: Phil Sutter <phil@nwl.cc>
Date: 2021-10-06 08:09:51
Hi Andrea, A remark regarding coding style: On Wed, Oct 06, 2021 at 12:08:04AM +0200, Andrea Claudi wrote: [...]
quoted hunk ↗ jump to hunk
diff --git a/configure b/configure index 7f4f3bd9..d57ce0f8 100755 --- a/configure +++ b/configure@@ -501,18 +501,30 @@ if [ $# -eq 1 ] && [ "$(echo $1 | cut -c 1)" != '-' ]; then else while true; do case "$1" in - --include_dir) - INCLUDE=$2 - shift 2 ;; - --libbpf_dir) - LIBBPF_DIR="$2" - shift 2 ;; - --libbpf_force) - if [ "$2" != 'on' ] && [ "$2" != 'off' ]; then + --include_dir | --include_dir=*)
So here the code combines the two cases,
+ INCLUDE="${1#*=}"
+ if [ "$INCLUDE" == "--include_dir" ]; then
just to fiddle it apart again. Did you consider leaving the old cases in
place and adding separate ones for the --opt=val cases like so:
| --include_dir=*)
| INCLUDE="${1#*=}"
| shift
| ;;
[...]+ --libbpf_force | --libbpf_force=*)
+ LIBBPF_FORCE="${1#*=}"
+ if [ "$LIBBPF_FORCE" == "--libbpf_force" ]; then
+ LIBBPF_FORCE="$2"
+ shift
+ fi
+ if [ "$LIBBPF_FORCE" != 'on' ] && [ "$LIBBPF_FORCE" != 'off' ]; thenTo avoid duplication here, I would move semantic checks into a second step. This would allow for things like: | --libbpf_force=invalid --libbpf_force=on but separating the syntactic parsing from semantic checks might be beneficial by itself, too. Cheers, Phil