| Message ID | 20170430232231.15151-3-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
On Sun, Apr 30, 2017 at 7:22 PM, Ben Pfaff <blp@ovn.org> wrote: > Until now, uuid_is_partial_string() returned the number of characters at > the beginning of a string that were the beginning of a valid UUID. This > is useful, but all of the callers actually wanted to get a value of 0 if > the string contained a character that was invalid for a UUID. This makes > that change. > > Examples: > > "123" previously yielded 3 and still does. > "xyzzy" previously yielded 0 and still does. > "123xyzzy" previously yielded 3, now yields 0. > "e66250bb-9531-491b-b9c3-5385cabb0080" previously yielded 36, still does. > "e66250bb-9531-491b-b9c3-5385cabb0080xyzzy" previously yielded 36, now 0. > > Signed-off-by: Ben Pfaff <blp@ovn.org> Acked-by: Russell Bryant <russell@ovn.org>
On Sun, Apr 30, 2017 at 4:22 PM, Ben Pfaff <blp@ovn.org> wrote: > Until now, uuid_is_partial_string() returned the number of characters at > the beginning of a string that were the beginning of a valid UUID. This > is useful, but all of the callers actually wanted to get a value of 0 if > the string contained a character that was invalid for a UUID. This makes > that change. > > Examples: > > "123" previously yielded 3 and still does. > "xyzzy" previously yielded 0 and still does. > "123xyzzy" previously yielded 3, now yields 0. > "e66250bb-9531-491b-b9c3-5385cabb0080" previously yielded 36, still does. > "e66250bb-9531-491b-b9c3-5385cabb0080xyzzy" previously yielded 36, now 0. > > Signed-off-by: Ben Pfaff <blp@ovn.org> Thanks for documenting those test cases in the commit message. It seems we don't have them in the actual unit tests. If you think it is worth while, I will be happy a add those tests and submit them in a separate patch. Acked-by: Andy Zhou <azhou@ovn.org>
On Tue, May 02, 2017 at 06:04:02PM -0700, Andy Zhou wrote: > On Sun, Apr 30, 2017 at 4:22 PM, Ben Pfaff <blp@ovn.org> wrote: > > Until now, uuid_is_partial_string() returned the number of characters at > > the beginning of a string that were the beginning of a valid UUID. This > > is useful, but all of the callers actually wanted to get a value of 0 if > > the string contained a character that was invalid for a UUID. This makes > > that change. > > > > Examples: > > > > "123" previously yielded 3 and still does. > > "xyzzy" previously yielded 0 and still does. > > "123xyzzy" previously yielded 3, now yields 0. > > "e66250bb-9531-491b-b9c3-5385cabb0080" previously yielded 36, still does. > > "e66250bb-9531-491b-b9c3-5385cabb0080xyzzy" previously yielded 36, now 0. > > > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > Thanks for documenting those test cases in the commit message. It > seems we don't have > them in the actual unit tests. If you think it is worth while, I will > be happy a add those tests > and submit them in a separate patch. > > Acked-by: Andy Zhou <azhou@ovn.org> Thanks Andy and Russell, I applied this to master. Andy, tests are always helpful, if you have time for to add some then please do, and thanks for that.
diff --git a/lib/db-ctl-base.c b/lib/db-ctl-base.c index ad98454c903d..ab617f9e065d 100644 --- a/lib/db-ctl-base.c +++ b/lib/db-ctl-base.c @@ -338,9 +338,7 @@ get_row(struct ctl_context *ctx, } } } - if (!row - && record_id[uuid_is_partial_string(record_id)] == '\0' - && strlen(record_id) >= 4) { + if (!row && uuid_is_partial_string(record_id) >= 4) { for (const struct ovsdb_idl_row *r = ovsdb_idl_first_row(ctx->idl, table); r != NULL; diff --git a/lib/uuid.c b/lib/uuid.c index a9094d36789a..636492bcbe5b 100644 --- a/lib/uuid.c +++ b/lib/uuid.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2008, 2009, 2010, 2011, 2013, 2016 Nicira, Inc. +/* Copyright (c) 2008, 2009, 2010, 2011, 2013, 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. @@ -211,22 +211,34 @@ error: return false; } -/* Returns the number of characters at the beginning of 's' that are valid for - * a UUID. For example, the "123" at the beginning of "123xyzzy" could begin a - * UUID, so uuid_is_partial_string() would return 3; for "xyzzy", this function - * would return 0, since "x" can't start a UUID. */ +/* If 's' is a string representation of a UUID, or the beginning of one, + * returns strlen(s), otherwise 0. + * + * For example: + * + * "123" yields 3 + * "xyzzy" yields 0 + * "123xyzzy" yields 0 + * "e66250bb-9531-491b-b9c3-5385cabb0080" yields 36 + * "e66250bb-9531-491b-b9c3-5385cabb0080xyzzy" yields 0 + */ int uuid_is_partial_string(const char *s) { static const char tmpl[UUID_LEN] = "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"; size_t i; for (i = 0; i < UUID_LEN; i++) { - if (tmpl[i] == 'x' - ? hexit_value(s[i]) < 0 - : s[i] != '-') { - break; + if (s[i] == '\0') { + return i; + } else if (tmpl[i] == 'x' + ? hexit_value(s[i]) < 0 + : s[i] != '-') { + return 0; } } + if (s[i] != '\0') { + return 0; + } return i; } diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c index ac292f316086..ac37d6014838 100644 --- a/ovn/utilities/ovn-sbctl.c +++ b/ovn/utilities/ovn-sbctl.c @@ -700,14 +700,14 @@ static char * parse_partial_uuid(char *s) { /* Accept a full or partial UUID. */ - if (uuid_is_partial_string(s) == strlen(s)) { + if (uuid_is_partial_string(s)) { return s; } /* Accept a full or partial UUID prefixed by 0x, since "ovs-ofctl * dump-flows" prints cookies prefixed by 0x. */ if (s[0] == '0' && (s[1] == 'x' || s[1] == 'X') - && uuid_is_partial_string(s + 2) == strlen(s + 2)) { + && uuid_is_partial_string(s + 2)) { return s + 2; }
Until now, uuid_is_partial_string() returned the number of characters at the beginning of a string that were the beginning of a valid UUID. This is useful, but all of the callers actually wanted to get a value of 0 if the string contained a character that was invalid for a UUID. This makes that change. Examples: "123" previously yielded 3 and still does. "xyzzy" previously yielded 0 and still does. "123xyzzy" previously yielded 3, now yields 0. "e66250bb-9531-491b-b9c3-5385cabb0080" previously yielded 36, still does. "e66250bb-9531-491b-b9c3-5385cabb0080xyzzy" previously yielded 36, now 0. Signed-off-by: Ben Pfaff <blp@ovn.org> --- lib/db-ctl-base.c | 4 +--- lib/uuid.c | 30 +++++++++++++++++++++--------- ovn/utilities/ovn-sbctl.c | 4 ++-- 3 files changed, 24 insertions(+), 14 deletions(-)