On Tue, 29 May 2018 16:57:07 +0200
Patrick Talbert <ptalb...@redhat.com> wrote:
> As mentioned in the ip-address man page, an address label must
> be equal to the device name or prefixed by the device name
> followed by a colon. Currently the only check on this input is
> to see if the device name appears at the beginning of the label
> string.
>
> This commit adds an additional check to ensure label == dev or
> continues with a colon.
>
> Signed-off-by: Patrick Talbert <ptalb...@redhat.com>
> Suggested-by: Stephen Hemminger <step...@networkplumber.org>
Yes, this looks better but still have some feedback.
> ---
> ip/ipaddress.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/ip/ipaddress.c b/ip/ipaddress.c
> index 00da14c..fce2008 100644
> --- a/ip/ipaddress.c
> +++ b/ip/ipaddress.c
> @@ -2040,6 +2040,22 @@ static bool ipaddr_is_multicast(inet_prefix *a)
> return false;
> }
>
> +static bool is_valid_label(const char *dev, const char *label)
> +{
> + char alias[strlen(dev) + 1];
> +
> + if (strlen(label) < strlen(dev))
> + return false;
> +
> + strcpy(alias, dev);
> + strcat(alias, ":");
> + if (strncmp(label, dev, strlen(dev)) == 0 ||
> + strncmp(label, alias, strlen(alias)) == 0)
> + return true;
> + else
> + return false;
> +}
This string copying and comparison still is much more overhead than it
needs to be. The following tests out to be equivalent with a single strncmp
and strlen.
Why not just:
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 00da14c6f97c..eac489e94fe4 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -2040,6 +2040,16 @@ static bool ipaddr_is_multicast(inet_prefix *a)
return false;
}
+static bool is_valid_label(const char *label, const char *dev)
+{
+ size_t len = strlen(dev);
+
+ if (strncmp(label, dev, len) != 0)
+ return false;
+
+ return label[len] == '\0' || label[len] == ':';
+}
+
Doesn't matter much now, but code seems to get copied.
