4:14 a.m.
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
4:14 a.m.
New subject: [libvirt] [PATCH 1/7] util: Make virStringHasCaseSuffix() return bool
It's a predicate, so bool is the appropriate return type.
Signed-off-by: Andrea Bolognani <abologna(a)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
4:14 a.m.
New subject: [libvirt] [PATCH 2/7] util: Make virStringStripSuffix() return bool
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(a)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
5:38 a.m.
New subject: [libvirt] [PATCH 2/7] util: Make virStringStripSuffix() return bool
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(a)redhat.com>
---
src/util/virstring.c | 8 ++++----
src/util/virstring.h | 4 ++--
tests/testutilsqemu.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
ACK
4:14 a.m.
New subject: [libvirt] [PATCH 3/7] util: Make virStringMatchesNameSuffix() return bool
It's a predicate, so bool is the appropriate return type.
Signed-off-by: Andrea Bolognani <abologna(a)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
5:38 a.m.
New subject: [libvirt] [PATCH 3/7] util: Make virStringMatchesNameSuffix() return bool
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(a)redhat.com>
---
src/util/virstring.c | 6 +++---
src/util/virstring.h | 6 +++---
2 files changed, 6 insertions(+), 6 deletions(-)
ACK
4:14 a.m.
New subject: [libvirt] [PATCH 4/7] util: Tweak virStringHasSuffix()
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(a)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
5:36 a.m.
New subject: [libvirt] [PATCH 4/7] util: Tweak virStringHasSuffix()
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(a)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
4:14 a.m.
New subject: [libvirt] [PATCH 5/7] util: Tweak virStringHasCaseSuffix()
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(a)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
4:14 a.m.
New subject: [libvirt] [PATCH 6/7] util: Tweak virStringMatchesNameSuffix()
Make it more similar to virStringStripSuffix().
Signed-off-by: Andrea Bolognani <abologna(a)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
4:14 a.m.
New subject: [libvirt] [PATCH 7/7] util: Tweak virStringMatchesNameSuffix() some more
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(a)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
10:40 a.m.
New subject: [libvirt] [PATCH 7/7] util: Tweak virStringMatchesNameSuffix() some more
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(a)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.
2090
days inactive
2090
days old
12 comments
2 participants
participants (2)
-
Andrea Bolognani -
Peter Krempa