[libvirt] [PATCH 0/7] util: Further tweaks to virstring
Some changes where suggested during review of https://www.redhat.com/archives/libvir-list/2019-March/msg00342.html and some weren't :) Andrea Bolognani (7): util: Make virStringHasCaseSuffix() return bool util: Make virStringStripSuffix() return bool util: Make virStringMatchesNameSuffix() return bool util: Tweak virStringHasSuffix() util: Tweak virStringHasCaseSuffix() util: Tweak virStringMatchesNameSuffix() util: Tweak virStringMatchesNameSuffix() some more src/util/virstring.c | 40 +++++++++++++++++++++++++--------------- src/util/virstring.h | 14 +++++++------- tests/testutilsqemu.c | 2 +- 3 files changed, 33 insertions(+), 23 deletions(-) -- 2.20.1
It's a predicate, so bool is the appropriate return type. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 4 ++-- src/util/virstring.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index cf3d9c6f03..cd781a84f9 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1248,7 +1248,7 @@ virStringHasSuffix(const char *str, return STREQ(str + len - suffixlen, suffix); } -int +bool virStringHasCaseSuffix(const char *str, const char *suffix) { @@ -1256,7 +1256,7 @@ virStringHasCaseSuffix(const char *str, int suffixlen = strlen(suffix); if (len < suffixlen) - return 0; + return false; return STRCASEEQ(str + len - suffixlen, suffix); } diff --git a/src/util/virstring.h b/src/util/virstring.h index 5b127cdba4..ed2db8a3dc 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -290,8 +290,8 @@ char *virStringReplace(const char *haystack, bool virStringHasSuffix(const char *str, const char *suffix); -int virStringHasCaseSuffix(const char *str, - const char *suffix); +bool virStringHasCaseSuffix(const char *str, + const char *suffix); int virStringStripSuffix(char *str, const char *suffix) ATTRIBUTE_RETURN_CHECK; int virStringMatchesNameSuffix(const char *file, -- 2.20.1
While this function is not, strictly speaking, a predicate, it still mostly behaves like one as evidenced by the vast majority of its callers, so using bool rather than int as the return type makes sense. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 8 ++++---- src/util/virstring.h | 4 ++-- tests/testutilsqemu.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index cd781a84f9..e479d3194c 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1261,7 +1261,7 @@ virStringHasCaseSuffix(const char *str, return STRCASEEQ(str + len - suffixlen, suffix); } -int +bool virStringStripSuffix(char *str, const char *suffix) { @@ -1269,14 +1269,14 @@ virStringStripSuffix(char *str, int suffixlen = strlen(suffix); if (len < suffixlen) - return 0; + return false; if (STRNEQ(str + len - suffixlen, suffix)) - return 0; + return false; str[len - suffixlen] = '\0'; - return 1; + return true; } int diff --git a/src/util/virstring.h b/src/util/virstring.h index ed2db8a3dc..f5f1ecbe1e 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -292,8 +292,8 @@ bool virStringHasSuffix(const char *str, const char *suffix); bool virStringHasCaseSuffix(const char *str, const char *suffix); -int virStringStripSuffix(char *str, - const char *suffix) ATTRIBUTE_RETURN_CHECK; +bool virStringStripSuffix(char *str, + const char *suffix) ATTRIBUTE_RETURN_CHECK; int virStringMatchesNameSuffix(const char *file, const char *name, const char *suffix); diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 61bf67d5ad..03a3f86c62 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -834,7 +834,7 @@ testQemuGetLatestCapsForArch(const char *dirname, if (rc == 0) continue; - if (virStringStripSuffix(tmp, fullsuffix) != 1) + if (!virStringStripSuffix(tmp, fullsuffix)) continue; if (virParseVersionString(tmp, &ver, false) < 0) { -- 2.20.1
On Thu, Mar 07, 2019 at 11:14:41 +0100, Andrea Bolognani wrote:
While this function is not, strictly speaking, a predicate, it still mostly behaves like one as evidenced by the vast majority of its callers, so using bool rather than int as the return type makes sense.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 8 ++++---- src/util/virstring.h | 4 ++-- tests/testutilsqemu.c | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-)
ACK
It's a predicate, so bool is the appropriate return type. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 6 +++--- src/util/virstring.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index e479d3194c..f669d59f58 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1279,7 +1279,7 @@ virStringStripSuffix(char *str, return true; } -int +bool virStringMatchesNameSuffix(const char *file, const char *name, const char *suffix) @@ -1291,9 +1291,9 @@ virStringMatchesNameSuffix(const char *file, if (filelen == (namelen + suffixlen) && STREQLEN(file, name, namelen) && STREQLEN(file + namelen, suffix, suffixlen)) - return 1; + return true; else - return 0; + return false; } /** diff --git a/src/util/virstring.h b/src/util/virstring.h index f5f1ecbe1e..1e36ac459c 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -294,9 +294,9 @@ bool virStringHasCaseSuffix(const char *str, const char *suffix); bool virStringStripSuffix(char *str, const char *suffix) ATTRIBUTE_RETURN_CHECK; -int virStringMatchesNameSuffix(const char *file, - const char *name, - const char *suffix); +bool virStringMatchesNameSuffix(const char *file, + const char *name, + const char *suffix); void virStringStripIPv6Brackets(char *str); bool virStringHasChars(const char *str, -- 2.20.1
On Thu, Mar 07, 2019 at 11:14:42 +0100, Andrea Bolognani wrote:
It's a predicate, so bool is the appropriate return type.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 6 +++--- src/util/virstring.h | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)
ACK
Make it more similar to virStringStripSuffix(), which also results in types being more correct since STRNEQ() returns int rather than bool. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index f669d59f58..a938669b63 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1245,7 +1245,10 @@ virStringHasSuffix(const char *str, if (len < suffixlen) return false; - return STREQ(str + len - suffixlen, suffix); + if (STRNEQ(str + len - suffixlen, suffix)) + return false; + + return true; } bool -- 2.20.1
On Thu, Mar 07, 2019 at 11:14:43 +0100, Andrea Bolognani wrote:
Make it more similar to virStringStripSuffix(), which also results in types being more correct since STRNEQ() returns int rather than bool.
# define STREQ(a, b) (strcmp(a, b) == 0) # define STRNEQ(a, b) (strcmp(a, b) != 0)
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/util/virstring.c b/src/util/virstring.c index f669d59f58..a938669b63 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1245,7 +1245,10 @@ virStringHasSuffix(const char *str, if (len < suffixlen) return false;
- return STREQ(str + len - suffixlen, suffix); + if (STRNEQ(str + len - suffixlen, suffix)) + return false; + + return true;
NACK
Make it more similar to virStringStripSuffix(), which also results in types being more correct since STRCASEEQ() returns int rather than bool. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index a938669b63..e47d76accd 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1261,7 +1261,10 @@ virStringHasCaseSuffix(const char *str, if (len < suffixlen) return false; - return STRCASEEQ(str + len - suffixlen, suffix); + if (STRCASENEQ(str + len - suffixlen, suffix)) + return false; + + return true; } bool -- 2.20.1
Make it more similar to virStringStripSuffix(). Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index e47d76accd..ba36562f85 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1294,12 +1294,16 @@ virStringMatchesNameSuffix(const char *file, int namelen = strlen(name); int suffixlen = strlen(suffix); - if (filelen == (namelen + suffixlen) && - STREQLEN(file, name, namelen) && - STREQLEN(file + namelen, suffix, suffixlen)) - return true; - else + if (filelen != (namelen + suffixlen)) + return false; + + if (STRNEQLEN(file, name, namelen)) return false; + + if (STRNEQLEN(file + namelen, suffix, suffixlen)) + return false; + + return true; } /** -- 2.20.1
We can use STRNEQ() instead of STRNEQLEN() since we're only interested in the trailing part of the string. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virstring.c b/src/util/virstring.c index ba36562f85..f23daca0bd 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1300,7 +1300,7 @@ virStringMatchesNameSuffix(const char *file, if (STRNEQLEN(file, name, namelen)) return false; - if (STRNEQLEN(file + namelen, suffix, suffixlen)) + if (STRNEQ(file + namelen, suffix)) return false; return true; -- 2.20.1
On Thu, Mar 07, 2019 at 11:14:46 +0100, Andrea Bolognani wrote:
We can use STRNEQ() instead of STRNEQLEN() since we're only interested in the trailing part of the string.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virstring.c b/src/util/virstring.c index ba36562f85..f23daca0bd 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1300,7 +1300,7 @@ virStringMatchesNameSuffix(const char *file, if (STRNEQLEN(file, name, namelen)) return false;
- if (STRNEQLEN(file + namelen, suffix, suffixlen)) + if (STRNEQ(file + namelen, suffix)) return false;
ACK to this if you rebase it without the previous patches. The change is okay as we verify that filelen == (namelen + suffixlen) prior to attempting this.
participants (2)
-
Andrea Bolognani -
Peter Krempa