Netdev List
 help / color / mirror / Atom feed
From: Phil Sutter <phil@nwl.cc>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: netdev@vger.kernel.org
Subject: [iproute PATCH v2] ip-address: Fix filtering by negated address flags
Date: Thu, 15 Nov 2018 14:28:59 +0100	[thread overview]
Message-ID: <20181115132859.15495-1-phil@nwl.cc> (raw)

When disabling a flag, one needs to AND with the inverse not the flag
itself. Otherwise specifying for instance 'home -nodad' will effectively
clear the flags variable.

While being at it, simplify the code a bit by merging common parts of
negated and non-negated case branches. Also allow for the "special
cases" to be inverted, too.

Fixes: f73ac674d0abf ("ip: change flag names to an array")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Improve "special cases" handling per Stephen's suggestion.
- Update man page accordingly.
---
 ip/ipaddress.c           | 43 +++++++++++++++++++---------------------
 man/man8/ip-address.8.in | 39 +++++++++++++++++++++++++++---------
 2 files changed, 50 insertions(+), 32 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index cd8cc76a3473f..7212f082b31b4 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -1211,38 +1211,35 @@ static void print_ifa_flags(FILE *fp, const struct ifaddrmsg *ifa,
 
 static int get_filter(const char *arg)
 {
+	bool inv = false;
 	unsigned int i;
 
+	if (arg[0] == '-') {
+		inv = true;
+		arg++;
+	}
+
 	/* Special cases */
 	if (strcmp(arg, "dynamic") == 0) {
-		filter.flags &= ~IFA_F_PERMANENT;
-		filter.flagmask |= IFA_F_PERMANENT;
+		inv = !inv;
+		arg = "permanent";
 	} else if (strcmp(arg, "primary") == 0) {
-		filter.flags &= ~IFA_F_SECONDARY;
-		filter.flagmask |= IFA_F_SECONDARY;
-	} else if (*arg == '-') {
-		for (i = 0; i < ARRAY_SIZE(ifa_flag_names); i++) {
-			if (strcmp(arg + 1, ifa_flag_names[i].name))
-				continue;
+		inv = !inv;
+		arg = "secondary";
+	}
 
-			filter.flags &= ifa_flag_names[i].value;
-			filter.flagmask |= ifa_flag_names[i].value;
-			return 0;
-		}
+	for (i = 0; i < ARRAY_SIZE(ifa_flag_names); i++) {
+		if (strcmp(arg, ifa_flag_names[i].name))
+			continue;
 
-		return -1;
-	} else {
-		for (i = 0; i < ARRAY_SIZE(ifa_flag_names); i++) {
-			if (strcmp(arg, ifa_flag_names[i].name))
-				continue;
+		if (inv)
+			filter.flags &= ~ifa_flag_names[i].value;
+		else
 			filter.flags |= ifa_flag_names[i].value;
-			filter.flagmask |= ifa_flag_names[i].value;
-			return 0;
-		}
-		return -1;
+		filter.flagmask |= ifa_flag_names[i].value;
+		return 0;
 	}
-
-	return 0;
+	return -1;
 }
 
 static int ifa_label_match_rta(int ifindex, const struct rtattr *rta)
diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
index c3861b3725ccb..2a553190a37e9 100644
--- a/man/man8/ip-address.8.in
+++ b/man/man8/ip-address.8.in
@@ -76,10 +76,15 @@ ip-address \- protocol address management
 .IR FLAG-LIST " := [ "  FLAG-LIST " ] " FLAG
 
 .ti -8
-.IR FLAG " := "
-.RB "[ " permanent " | " dynamic " | " secondary " | " primary " |"
-.RB [ - ] tentative " | [" - ] deprecated " | [" - ] dadfailed " |"
-.BR temporary " |"
+.IR FLAG " := ["
+.RB [ - ] permanent " |"
+.RB [ - ] dynamic " |"
+.RB [ - ] secondary " |"
+.RB [ - ] primary " |"
+.RB [ - ] tentative " |"
+.RB [ - ] deprecated " |"
+.RB [ - ] dadfailed " |"
+.RB [ - ] temporary " |"
 .IR CONFFLAG-LIST " ]"
 
 .ti -8
@@ -334,7 +339,9 @@ only list running interfaces.
 .BR dynamic " and " permanent
 (IPv6 only) only list addresses installed due to stateless
 address configuration or only list permanent (not dynamic)
-addresses.
+addresses. These two flags are inverses of each other, so
+.BR -dynamic " is equal to " permanent " and "
+.BR -permanent " is equal to " dynamic .
 
 .TP
 .B tentative
@@ -365,12 +372,26 @@ address detection.
 address detection.
 
 .TP
-.B temporary
-(IPv6 only) only list temporary addresses.
+.BR temporary " or " secondary
+List temporary IPv6 or secondary IPv4 addresses only. The Linux kernel shares a
+single bit for those, so they are actually aliases for each other although the
+meaning differs depending on address family.
 
 .TP
-.BR primary " and " secondary
-only list primary (or secondary) addresses.
+.BR -temporary " or " -secondary
+These flags are aliases for
+.BR primary .
+
+.TP
+.B primary
+List only primary addresses, in IPv6 exclude temporary ones. This flag is the
+inverse of
+.BR temporary " and " secondary .
+
+.TP
+.B -primary
+This is an alias for
+.BR temporary " or " secondary .
 
 .SS ip address flush - flush protocol addresses
 This command flushes the protocol addresses selected by some criteria.
-- 
2.19.0

                 reply	other threads:[~2018-11-15 23:37 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181115132859.15495-1-phil@nwl.cc \
    --to=phil@nwl.cc \
    --cc=netdev@vger.kernel.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox