[ovs-dev,03/27] uuid: New function uuid_is_partial_match().

On Sun, Apr 30, 2017 at 4:22 PM, Ben Pfaff <blp@ovn.org> wrote:
> This will have another caller in an upcoming commit.
>
> Signed-off-by: Ben Pfaff <blp@ovn.org>

 Acked-by: Andy Zhou <azhou@ovn.org>

> ---
>  lib/db-ctl-base.c | 10 +---------
>  lib/uuid.c        | 11 +++++++++++
>  lib/uuid.h        |  3 ++-
>  3 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c
> index ab617f9e065d..18109691b1d6 100644
> --- a/lib/db-ctl-base.c
> +++ b/lib/db-ctl-base.c
> @@ -300,14 +300,6 @@ get_row_by_id(struct ctl_context *ctx,
>      return final;
>  }
>
> -static bool
> -is_partial_uuid_match(const struct uuid *uuid, const char *match)
> -{
> -    char uuid_s[UUID_LEN + 1];
> -    snprintf(uuid_s, sizeof uuid_s, UUID_FMT, UUID_ARGS(uuid));
> -    return !strncmp(uuid_s, match, strlen(match));
> -}
> -
>  static const struct ovsdb_idl_row *
>  get_row(struct ctl_context *ctx,
>          const struct ovsdb_idl_table_class *table, const char *record_id,
> @@ -343,7 +335,7 @@ get_row(struct ctl_context *ctx,
>                                                                   table);
>               r != NULL;
>               r = ovsdb_idl_next_row(r)) {
> -            if (is_partial_uuid_match(&r->uuid, record_id)) {
> +            if (uuid_is_partial_match(&r->uuid, record_id) >= 4) {
>                  if (!row) {
>                      row = r;
>                  } else {
> diff --git a/lib/uuid.c b/lib/uuid.c
> index 636492bcbe5b..0b20c24faecd 100644
> --- a/lib/uuid.c
> +++ b/lib/uuid.c
> @@ -242,6 +242,17 @@ uuid_is_partial_string(const char *s)
>      return i;
>  }
>
> +/* Compares 'match' to the string representation of 'uuid'.  If 'match' equals
> + * or is a prefix of this string representation, returns strlen(match);
> + * otherwise, returns 0.  (Thus, the caller can  */

The last sentence is not finished.

> +int
> +uuid_is_partial_match(const struct uuid *uuid, const char *match)
> +{
> +    char uuid_s[UUID_LEN + 1];
> +    snprintf(uuid_s, sizeof uuid_s, UUID_FMT, UUID_ARGS(uuid));
> +    size_t match_len = strlen(match);
> +    return !strncmp(uuid_s, match, match_len) ? match_len : 0;
> +}

I am not sure if the following optimization makes sense, since the main use case
if mainly for match to be << UUID_LEN.  On the other hand, it is cheap to check.

>
>  static void
>  sha1_update_int(struct sha1_ctx *sha1_ctx, uintmax_t x)
> diff --git a/lib/uuid.h b/lib/uuid.h
> index 605ec17b7455..10bc8b541bac 100644
> --- a/lib/uuid.h
> +++ b/lib/uuid.h
> @@ -1,4 +1,4 @@
> -/* Copyright (c) 2008, 2009, 2010, 2016 Nicira, Inc.
> +/* Copyright (c) 2008, 2009, 2010, 2016, 2017 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -65,6 +65,7 @@ int uuid_compare_3way(const struct uuid *, const struct uuid *);
>  bool uuid_from_string(struct uuid *, const char *);
>  bool uuid_from_string_prefix(struct uuid *, const char *);
>  int uuid_is_partial_string(const char *);
> +int uuid_is_partial_match(const struct uuid *, const char *match);
>  void uuid_set_bits_v4(struct uuid *);
>
>  #endif /* uuid.h */
> --
> 2.10.2
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

On Tue, May 02, 2017 at 07:06:06PM -0700, Andy Zhou wrote:
> On Sun, Apr 30, 2017 at 4:22 PM, Ben Pfaff <blp@ovn.org> wrote:
> > This will have another caller in an upcoming commit.
> >
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> 
>  Acked-by: Andy Zhou <azhou@ovn.org>

Thanks!

...

> > diff --git a/lib/uuid.c b/lib/uuid.c
> > index 636492bcbe5b..0b20c24faecd 100644
> > --- a/lib/uuid.c
> > +++ b/lib/uuid.c
> > @@ -242,6 +242,17 @@ uuid_is_partial_string(const char *s)
> >      return i;
> >  }
> >
> > +/* Compares 'match' to the string representation of 'uuid'.  If 'match' equals
> > + * or is a prefix of this string representation, returns strlen(match);
> > + * otherwise, returns 0.  (Thus, the caller can  */
> 
> The last sentence is not finished.

Oops.  Now I've deleted it.

> > +int
> > +uuid_is_partial_match(const struct uuid *uuid, const char *match)
> > +{
> > +    char uuid_s[UUID_LEN + 1];
> > +    snprintf(uuid_s, sizeof uuid_s, UUID_FMT, UUID_ARGS(uuid));
> > +    size_t match_len = strlen(match);
> > +    return !strncmp(uuid_s, match, match_len) ? match_len : 0;
> > +}
> 
> I am not sure if the following optimization makes sense, since the
> main use case if mainly for match to be << UUID_LEN.  On the other
> hand, it is cheap to check.

Thanks for the suggestion.

My own instinct is that the expensive part here is probably the
snprintf().  I think I'll leave this as it is until we find that there's
a performance problem of some kind.

> diff --git a/lib/uuid.c b/lib/uuid.c
> index 0b20c24faecd..5fa7ed91e852 100644
> --- a/lib/uuid.c
> +++ b/lib/uuid.c
> @@ -249,8 +249,11 @@ int
>  uuid_is_partial_match(const struct uuid *uuid, const char *match)
>  {
>      char uuid_s[UUID_LEN + 1];
> -    snprintf(uuid_s, sizeof uuid_s, UUID_FMT, UUID_ARGS(uuid));
>      size_t match_len = strlen(match);
> +    if (match_len > UUID_LEN) {
> +        return 0;
> +    }
> +    snprintf(uuid_s, sizeof uuid_s, UUID_FMT, UUID_ARGS(uuid));
>      return !strncmp(uuid_s, match, match_len) ? match_len : 0;
>  }

Thanks again, I'll apply this in a minute.