[libvirt] [PATCH 0/5] util: Move some functions from virfile to virstring

Andrea Bolognani (5): util: Rename virFileHasSuffix() to virStringHasCaseSuffix() util: Rename virFileStripSuffix() to virStringStripSuffix() util: Rename virFileMatchesNameSuffix() to virStringMatchesNameSuffix() util: Add virStringHasSuffix() Use virStringHasSuffix() where possible src/conf/virdomainobjlist.c | 2 +- src/conf/virnetworkobj.c | 4 +- src/conf/virnwfilterbindingobjlist.c | 2 +- src/conf/virnwfilterobj.c | 2 +- src/conf/virsecretobj.c | 4 +- src/conf/virstorageobj.c | 6 +-- src/esx/esx_driver.c | 2 +- src/esx/esx_storage_backend_vmfs.c | 4 +- src/libvirt_private.syms | 7 ++-- src/util/virfile.c | 48 ---------------------- src/util/virfile.h | 10 ----- src/util/virstoragefile.c | 2 +- src/util/virstring.c | 60 ++++++++++++++++++++++++++++ src/util/virstring.h | 10 +++++ src/vmware/vmware_conf.c | 2 +- src/vmx/vmx.c | 12 +++--- tests/testutils.c | 4 +- tests/testutilsqemu.c | 2 +- tests/virschematest.c | 4 +- tools/nss/libvirt_nss.c | 4 +- 20 files changed, 102 insertions(+), 89 deletions(-) -- 2.20.1

Despite its name, this is really just a general-purpose string manipulation function, so it should be moved to the virstring module and renamed accordingly. In addition to the obvious s/File/String/, also tweak the name to make it clear that the presence of the suffix is verified using case-insensitive comparison. A few trivial whitespace changes are squashed in. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/virsecretobj.c | 2 +- src/conf/virstorageobj.c | 2 +- src/esx/esx_driver.c | 2 +- src/esx/esx_storage_backend_vmfs.c | 4 ++-- src/libvirt_private.syms | 2 +- src/util/virfile.c | 13 ------------- src/util/virfile.h | 3 --- src/util/virstoragefile.c | 2 +- src/util/virstring.c | 12 ++++++++++++ src/util/virstring.h | 3 +++ src/vmware/vmware_conf.c | 2 +- src/vmx/vmx.c | 12 ++++++------ tests/testutils.c | 4 ++-- tests/virschematest.c | 4 ++-- tools/nss/libvirt_nss.c | 4 ++-- 15 files changed, 35 insertions(+), 36 deletions(-) diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 8b418d5e66..5cd36b8a40 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -949,7 +949,7 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets, char *path; virSecretObjPtr obj; - if (!virFileHasSuffix(de->d_name, ".xml")) + if (!virStringHasCaseSuffix(de->d_name, ".xml")) continue; if (!(path = virFileBuildPath(configDir, de->d_name, NULL))) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 2286857acf..68c89bd9a9 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1719,7 +1719,7 @@ virStoragePoolObjLoadAllConfigs(virStoragePoolObjListPtr pools, char *autostartLink; virStoragePoolObjPtr obj; - if (!virFileHasSuffix(entry->d_name, ".xml")) + if (!virStringHasCaseSuffix(entry->d_name, ".xml")) continue; if (!(path = virFileBuildPath(configDir, entry->d_name, NULL))) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 379c2bae73..afebd78566 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3078,7 +3078,7 @@ esxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) goto cleanup; } - if (! virFileHasSuffix(src, ".vmdk")) { + if (!virStringHasCaseSuffix(src, ".vmdk")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting source '%s' of first file-based harddisk to " "be a VMDK image"), src); diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 6d6bd1a6ce..c56f887c25 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -884,7 +884,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, goto cleanup; } - if (! virFileHasSuffix(def->name, ".vmdk")) { + if (!virStringHasCaseSuffix(def->name, ".vmdk")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Volume name '%s' has unsupported suffix, " "expecting '.vmdk'"), def->name); @@ -1104,7 +1104,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, goto cleanup; } - if (! virFileHasSuffix(def->name, ".vmdk")) { + if (!virStringHasCaseSuffix(def->name, ".vmdk")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Volume name '%s' has unsupported suffix, " "expecting '.vmdk'"), def->name); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 69643732e0..e4ed1def33 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1834,7 +1834,6 @@ virFileGetHugepageSize; virFileGetMountReverseSubtree; virFileGetMountSubtree; virFileGetXAttr; -virFileHasSuffix; virFileInData; virFileIsAbsPath; virFileIsCDROM; @@ -2958,6 +2957,7 @@ virStrdup; virStringBufferIsPrintable; virStringEncodeBase64; virStringFilterChars; +virStringHasCaseSuffix; virStringHasChars; virStringHasControlChars; virStringIsEmpty; diff --git a/src/util/virfile.c b/src/util/virfile.c index 9208523c47..21f7dc1ac3 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1543,19 +1543,6 @@ virFileMatchesNameSuffix(const char *file, return 0; } -int -virFileHasSuffix(const char *str, - const char *suffix) -{ - int len = strlen(str); - int suffixlen = strlen(suffix); - - if (len < suffixlen) - return 0; - - return STRCASEEQ(str + len - suffixlen, suffix); -} - #define SAME_INODE(Stat_buf_1, Stat_buf_2) \ ((Stat_buf_1).st_ino == (Stat_buf_2).st_ino \ && (Stat_buf_1).st_dev == (Stat_buf_2).st_dev) diff --git a/src/util/virfile.h b/src/util/virfile.h index 0b946fe1ca..5a43d57181 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -166,9 +166,6 @@ int virFileMatchesNameSuffix(const char *file, const char *name, const char *suffix); -int virFileHasSuffix(const char *str, - const char *suffix); - int virFileStripSuffix(char *str, const char *suffix) ATTRIBUTE_RETURN_CHECK; diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 233278e879..50ee9b574c 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -725,7 +725,7 @@ virStorageFileMatchesExtension(const char *extension, if (extension == NULL) return false; - if (virFileHasSuffix(path, extension)) + if (virStringHasCaseSuffix(path, extension)) return true; return false; diff --git a/src/util/virstring.c b/src/util/virstring.c index 8a791f96d4..399b9468e6 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1235,6 +1235,18 @@ virStringReplace(const char *haystack, return virBufferContentAndReset(&buf); } +int +virStringHasCaseSuffix(const char *str, + const char *suffix) +{ + int len = strlen(str); + int suffixlen = strlen(suffix); + + if (len < suffixlen) + return 0; + + return STRCASEEQ(str + len - suffixlen, suffix); +} /** * virStringStripIPv6Brackets: diff --git a/src/util/virstring.h b/src/util/virstring.h index f2e72936c8..b2ba97a8d1 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -288,6 +288,9 @@ char *virStringReplace(const char *haystack, const char *newneedle) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int virStringHasCaseSuffix(const char *str, + const char *suffix); + void virStringStripIPv6Brackets(char *str); bool virStringHasChars(const char *str, const char *chars); diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 21559da4a4..0c1b1f9550 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -434,7 +434,7 @@ vmwareVmxPath(virDomainDefPtr vmdef, char **vmxPath) if (vmwareParsePath(src, &directoryName, &fileName) < 0) goto cleanup; - if (!virFileHasSuffix(fileName, ".vmdk")) { + if (!virStringHasCaseSuffix(fileName, ".vmdk")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Expecting source '%s' of first file-based harddisk " "to be a VMDK image"), src); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 75cdea9067..bbf8e55c3f 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1600,7 +1600,7 @@ virVMXParseConfig(virVMXContext *ctx, if (virVMXGetConfigString(conf, "guestOS", &guestOS, true) < 0) goto cleanup; - if (guestOS != NULL && virFileHasSuffix(guestOS, "-64")) { + if (guestOS != NULL && virStringHasCaseSuffix(guestOS, "-64")) { def->os.arch = VIR_ARCH_X86_64; } else { def->os.arch = VIR_ARCH_I686; @@ -2218,7 +2218,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con /* Setup virDomainDiskDef */ if (device == VIR_DOMAIN_DISK_DEVICE_DISK) { - if (virFileHasSuffix(fileName, ".vmdk")) { + if (virStringHasCaseSuffix(fileName, ".vmdk")) { char *tmp; if (deviceType != NULL) { @@ -2254,7 +2254,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con if (mode) (*def)->transient = STRCASEEQ(mode, "independent-nonpersistent"); - } else if (virFileHasSuffix(fileName, ".iso") || + } else if (virStringHasCaseSuffix(fileName, ".iso") || STREQ(fileName, "emptyBackingString") || (deviceType && (STRCASEEQ(deviceType, "atapi-cdrom") || @@ -2277,7 +2277,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto cleanup; } } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) { - if (virFileHasSuffix(fileName, ".iso")) { + if (virStringHasCaseSuffix(fileName, ".iso")) { char *tmp; if (deviceType && STRCASENEQ(deviceType, "cdrom-image")) { @@ -2295,7 +2295,7 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOptionPtr xmlopt, virConfPtr con goto cleanup; } VIR_FREE(tmp); - } else if (virFileHasSuffix(fileName, ".vmdk")) { + } else if (virStringHasCaseSuffix(fileName, ".vmdk")) { /* * This function was called in order to parse a CDROM device, but * .vmdk files are for harddisk devices only. Just ignore it, @@ -3585,7 +3585,7 @@ virVMXFormatDisk(virVMXContext *ctx, virDomainDiskDefPtr def, const char *src = virDomainDiskGetSource(def); if (src) { - if (!virFileHasSuffix(src, fileExt)) { + if (!virStringHasCaseSuffix(src, fileExt)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Image file for %s %s '%s' has " "unsupported suffix, expecting '%s'"), diff --git a/tests/testutils.c b/tests/testutils.c index 13bb9630df..cfdf122ae0 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -525,8 +525,8 @@ virTestRewrapFile(const char *filename) char *script = NULL; virCommandPtr cmd = NULL; - if (!(virFileHasSuffix(filename, ".args") || - virFileHasSuffix(filename, ".ldargs"))) + if (!(virStringHasCaseSuffix(filename, ".args") || + virStringHasCaseSuffix(filename, ".ldargs"))) return 0; if (!perl) { diff --git a/tests/virschematest.c b/tests/virschematest.c index d1bcdeac9c..46c67760a7 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -41,7 +41,7 @@ static int testSchemaFile(const void *args) { const struct testSchemaData *data = args; - bool shouldFail = virFileHasSuffix(data->xml_path, "-invalid.xml"); + bool shouldFail = virStringHasCaseSuffix(data->xml_path, "-invalid.xml"); xmlDocPtr xml = NULL; int ret = -1; @@ -82,7 +82,7 @@ testSchemaDir(const char *schema, return -1; while ((rc = virDirRead(dir, &ent, dir_path)) > 0) { - if (!virFileHasSuffix(ent->d_name, ".xml")) + if (!virStringHasCaseSuffix(ent->d_name, ".xml")) continue; if (ent->d_name[0] == '.') continue; diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index 46d051c08c..4d86f8e6ce 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -274,7 +274,7 @@ findLease(const char *name, while ((ret = virDirRead(dir, &entry, leaseDir)) > 0) { char *path; - if (virFileHasSuffix(entry->d_name, ".status")) { + if (virStringHasCaseSuffix(entry->d_name, ".status")) { if (!(path = virFileBuildPath(leaseDir, entry->d_name, NULL))) goto cleanup; @@ -285,7 +285,7 @@ findLease(const char *name, goto cleanup; } VIR_FREE(path); - } else if (virFileHasSuffix(entry->d_name, ".macs")) { + } else if (virStringHasCaseSuffix(entry->d_name, ".macs")) { if (!(path = virFileBuildPath(leaseDir, entry->d_name, NULL))) goto cleanup; -- 2.20.1

On Wed, Mar 06, 2019 at 17:54:00 +0100, Andrea Bolognani wrote:
Despite its name, this is really just a general-purpose string manipulation function, so it should be moved to the virstring module and renamed accordingly.
In addition to the obvious s/File/String/, also tweak the name to make it clear that the presence of the suffix is verified using case-insensitive comparison.
How about also adding a function comment with it?
A few trivial whitespace changes are squashed in.
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
ACK. You could also fix the return value type.

Despite its name, this is really just a general-purpose string manipulation function, so it should be moved to the virstring module and renamed accordingly. A few trivial whitespace changes are squashed in. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/virdomainobjlist.c | 2 +- src/conf/virnetworkobj.c | 4 ++-- src/conf/virnwfilterbindingobjlist.c | 2 +- src/conf/virnwfilterobj.c | 2 +- src/conf/virstorageobj.c | 2 +- src/libvirt_private.syms | 2 +- src/util/virfile.c | 18 ------------------ src/util/virfile.h | 3 --- src/util/virstring.c | 18 ++++++++++++++++++ src/util/virstring.h | 2 ++ tests/testutilsqemu.c | 2 +- 11 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 0e943d0a6c..7742de94f2 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -597,7 +597,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virDomainObjPtr dom; - if (!virFileStripSuffix(entry->d_name, ".xml")) + if (!virStringStripSuffix(entry->d_name, ".xml")) continue; /* NB: ignoring errors, so one malformed config doesn't diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index ae1264325a..c9336e0472 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1068,7 +1068,7 @@ virNetworkObjLoadAllState(virNetworkObjListPtr nets, while ((ret = virDirRead(dir, &entry, stateDir)) > 0) { virNetworkObjPtr obj; - if (!virFileStripSuffix(entry->d_name, ".xml")) + if (!virStringStripSuffix(entry->d_name, ".xml")) continue; obj = virNetworkLoadState(nets, stateDir, entry->d_name); @@ -1096,7 +1096,7 @@ virNetworkObjLoadAllConfigs(virNetworkObjListPtr nets, while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virNetworkObjPtr obj; - if (!virFileStripSuffix(entry->d_name, ".xml")) + if (!virStringStripSuffix(entry->d_name, ".xml")) continue; /* NB: ignoring errors, so one malformed config doesn't diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c index 887c7c7b09..06ccbf53af 100644 --- a/src/conf/virnwfilterbindingobjlist.c +++ b/src/conf/virnwfilterbindingobjlist.c @@ -304,7 +304,7 @@ virNWFilterBindingObjListLoadAllConfigs(virNWFilterBindingObjListPtr bindings, while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virNWFilterBindingObjPtr binding; - if (!virFileStripSuffix(entry->d_name, ".xml")) + if (!virStringStripSuffix(entry->d_name, ".xml")) continue; /* NB: ignoring errors, so one malformed config doesn't diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index 0136a0d56c..d2d957a1cc 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -547,7 +547,7 @@ virNWFilterObjListLoadAllConfigs(virNWFilterObjListPtr nwfilters, while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virNWFilterObjPtr obj; - if (!virFileStripSuffix(entry->d_name, ".xml")) + if (!virStringStripSuffix(entry->d_name, ".xml")) continue; obj = virNWFilterObjListLoadConfig(nwfilters, configDir, entry->d_name); diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 68c89bd9a9..85b6dd3695 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1688,7 +1688,7 @@ virStoragePoolObjLoadAllState(virStoragePoolObjListPtr pools, while ((ret = virDirRead(dir, &entry, stateDir)) > 0) { virStoragePoolObjPtr obj; - if (!virFileStripSuffix(entry->d_name, ".xml")) + if (!virStringStripSuffix(entry->d_name, ".xml")) continue; if (!(obj = virStoragePoolObjLoadState(pools, stateDir, entry->d_name))) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e4ed1def33..f94a8fdc3d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1882,7 +1882,6 @@ virFileSetACLs; virFileSetupDev; virFileSetXAttr; virFileSkipRoot; -virFileStripSuffix; virFileTouch; virFileUnlock; virFileUpdatePerm; @@ -2982,6 +2981,7 @@ virStringSplit; virStringSplitCount; virStringStripControlChars; virStringStripIPv6Brackets; +virStringStripSuffix; virStringToUpper; virStringTrimOptionalNewline; virStrncpy; diff --git a/src/util/virfile.c b/src/util/virfile.c index 21f7dc1ac3..c0f3f56293 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1044,24 +1044,6 @@ int virFileDeleteTree(const char *dir) return ret; } -int -virFileStripSuffix(char *str, const char *suffix) -{ - int len = strlen(str); - int suffixlen = strlen(suffix); - - if (len < suffixlen) - return 0; - - if (STRNEQ(str + len - suffixlen, suffix)) - return 0; - - str[len-suffixlen] = '\0'; - - return 1; -} - - /* Like read(), but restarts after EINTR. Doesn't play * nicely with nonblocking FD and EAGAIN, in which case * you want to use bare read(). Or even use virSocket() diff --git a/src/util/virfile.h b/src/util/virfile.h index 5a43d57181..0079e234f8 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -166,9 +166,6 @@ int virFileMatchesNameSuffix(const char *file, const char *name, const char *suffix); -int virFileStripSuffix(char *str, - const char *suffix) ATTRIBUTE_RETURN_CHECK; - int virFileLinkPointsTo(const char *checkLink, const char *checkDest) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/util/virstring.c b/src/util/virstring.c index 399b9468e6..60b4167af4 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1248,6 +1248,24 @@ virStringHasCaseSuffix(const char *str, return STRCASEEQ(str + len - suffixlen, suffix); } +int +virStringStripSuffix(char *str, + const char *suffix) +{ + int len = strlen(str); + int suffixlen = strlen(suffix); + + if (len < suffixlen) + return 0; + + if (STRNEQ(str + len - suffixlen, suffix)) + return 0; + + str[len - suffixlen] = '\0'; + + return 1; +} + /** * virStringStripIPv6Brackets: * @str: the string to strip diff --git a/src/util/virstring.h b/src/util/virstring.h index b2ba97a8d1..580e4da9b9 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -290,6 +290,8 @@ char *virStringReplace(const char *haystack, int virStringHasCaseSuffix(const char *str, const char *suffix); +int virStringStripSuffix(char *str, + const char *suffix) ATTRIBUTE_RETURN_CHECK; void virStringStripIPv6Brackets(char *str); bool virStringHasChars(const char *str, diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 1736bad032..61bf67d5ad 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -834,7 +834,7 @@ testQemuGetLatestCapsForArch(const char *dirname, if (rc == 0) continue; - if (virFileStripSuffix(tmp, fullsuffix) != 1) + if (virStringStripSuffix(tmp, fullsuffix) != 1) continue; if (virParseVersionString(tmp, &ver, false) < 0) { -- 2.20.1

On Wed, Mar 06, 2019 at 17:54:01 +0100, Andrea Bolognani wrote:
Despite its name, this is really just a general-purpose string manipulation function, so it should be moved to the virstring module and renamed accordingly.
A few trivial whitespace changes are squashed in.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/virdomainobjlist.c | 2 +- src/conf/virnetworkobj.c | 4 ++-- src/conf/virnwfilterbindingobjlist.c | 2 +- src/conf/virnwfilterobj.c | 2 +- src/conf/virstorageobj.c | 2 +- src/libvirt_private.syms | 2 +- src/util/virfile.c | 18 ------------------ src/util/virfile.h | 3 --- src/util/virstring.c | 18 ++++++++++++++++++ src/util/virstring.h | 2 ++ tests/testutilsqemu.c | 2 +- 11 files changed, 28 insertions(+), 29 deletions(-)
ACK. You could also fix the return value type.

Despite its name, this is really just a general-purpose string manipulation function, so it should be moved to the virstring module and renamed accordingly. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/virsecretobj.c | 2 +- src/conf/virstorageobj.c | 2 +- src/libvirt_private.syms | 2 +- src/util/virfile.c | 17 ----------------- src/util/virfile.h | 4 ---- src/util/virstring.c | 17 +++++++++++++++++ src/util/virstring.h | 3 +++ 7 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 5cd36b8a40..b01dc7cc70 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -819,7 +819,7 @@ virSecretLoadValidateUUID(virSecretDefPtr def, virUUIDFormat(def->uuid, uuidstr); - if (!virFileMatchesNameSuffix(file, uuidstr, ".xml")) { + if (!virStringMatchesNameSuffix(file, uuidstr, ".xml")) { virReportError(VIR_ERR_INTERNAL_ERROR, _("<uuid> does not match secret file name '%s'"), file); diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 85b6dd3695..a30d86d070 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1585,7 +1585,7 @@ virStoragePoolObjLoad(virStoragePoolObjListPtr pools, if (!(def = virStoragePoolDefParseFile(path))) return NULL; - if (!virFileMatchesNameSuffix(file, def->name, ".xml")) { + if (!virStringMatchesNameSuffix(file, def->name, ".xml")) { virReportError(VIR_ERR_XML_ERROR, _("Storage pool config filename '%s' does " "not match pool name '%s'"), diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f94a8fdc3d..d7ccd509e9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1851,7 +1851,6 @@ virFileLoopDeviceAssociate; virFileMakeParentPath; virFileMakePath; virFileMakePathWithMode; -virFileMatchesNameSuffix; virFileMoveMount; virFileNBDDeviceAssociate; virFileOpenAs; @@ -2972,6 +2971,7 @@ virStringListLength; virStringListMerge; virStringListRemove; virStringMatch; +virStringMatchesNameSuffix; virStringParsePort; virStringReplace; virStringSearch; diff --git a/src/util/virfile.c b/src/util/virfile.c index c0f3f56293..a4c6d184af 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -1508,23 +1508,6 @@ virFileWriteStr(const char *path, const char *str, mode_t mode) return 0; } -int -virFileMatchesNameSuffix(const char *file, - const char *name, - const char *suffix) -{ - int filelen = strlen(file); - int namelen = strlen(name); - int suffixlen = strlen(suffix); - - if (filelen == (namelen + suffixlen) && - STREQLEN(file, name, namelen) && - STREQLEN(file + namelen, suffix, suffixlen)) - return 1; - else - return 0; -} - #define SAME_INODE(Stat_buf_1, Stat_buf_2) \ ((Stat_buf_1).st_ino == (Stat_buf_2).st_ino \ && (Stat_buf_1).st_dev == (Stat_buf_2).st_dev) diff --git a/src/util/virfile.h b/src/util/virfile.h index 0079e234f8..3dedb7666a 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -162,10 +162,6 @@ int virFileReadBufQuiet(const char *file, char *buf, int len) int virFileWriteStr(const char *path, const char *str, mode_t mode) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -int virFileMatchesNameSuffix(const char *file, - const char *name, - const char *suffix); - int virFileLinkPointsTo(const char *checkLink, const char *checkDest) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/util/virstring.c b/src/util/virstring.c index 60b4167af4..b4d10f9884 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1266,6 +1266,23 @@ virStringStripSuffix(char *str, return 1; } +int +virStringMatchesNameSuffix(const char *file, + const char *name, + const char *suffix) +{ + int filelen = strlen(file); + int namelen = strlen(name); + int suffixlen = strlen(suffix); + + if (filelen == (namelen + suffixlen) && + STREQLEN(file, name, namelen) && + STREQLEN(file + namelen, suffix, suffixlen)) + return 1; + else + return 0; +} + /** * virStringStripIPv6Brackets: * @str: the string to strip diff --git a/src/util/virstring.h b/src/util/virstring.h index 580e4da9b9..69030566e9 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -292,6 +292,9 @@ int virStringHasCaseSuffix(const char *str, const char *suffix); int virStringStripSuffix(char *str, const char *suffix) ATTRIBUTE_RETURN_CHECK; +int virStringMatchesNameSuffix(const char *file, + const char *name, + const char *suffix); void virStringStripIPv6Brackets(char *str); bool virStringHasChars(const char *str, -- 2.20.1

On Wed, Mar 06, 2019 at 17:54:02 +0100, Andrea Bolognani wrote:
Despite its name, this is really just a general-purpose string manipulation function, so it should be moved to the virstring module and renamed accordingly.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/virsecretobj.c | 2 +- src/conf/virstorageobj.c | 2 +- src/libvirt_private.syms | 2 +- src/util/virfile.c | 17 ----------------- src/util/virfile.h | 4 ---- src/util/virstring.c | 17 +++++++++++++++++ src/util/virstring.h | 3 +++ 7 files changed, 23 insertions(+), 24 deletions(-)
ACK. Although you could also fix the return value type when at it.

This is the case-sensitive counterpart of the existing virStringHasCaseSuffix() function. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 13 +++++++++++++ src/util/virstring.h | 2 ++ 2 files changed, 15 insertions(+) diff --git a/src/util/virstring.c b/src/util/virstring.c index b4d10f9884..b9b33e3d1a 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1235,6 +1235,19 @@ virStringReplace(const char *haystack, return virBufferContentAndReset(&buf); } +int +virStringHasSuffix(const char *str, + const char *suffix) +{ + int len = strlen(str); + int suffixlen = strlen(suffix); + + if (len < suffixlen) + return 0; + + return STREQ(str + len - suffixlen, suffix); +} + int virStringHasCaseSuffix(const char *str, const char *suffix) diff --git a/src/util/virstring.h b/src/util/virstring.h index 69030566e9..dff21d9cf1 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -288,6 +288,8 @@ char *virStringReplace(const char *haystack, const char *newneedle) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3); +int virStringHasSuffix(const char *str, + const char *suffix); int virStringHasCaseSuffix(const char *str, const char *suffix); int virStringStripSuffix(char *str, -- 2.20.1

On Wed, Mar 06, 2019 at 17:54:03 +0100, Andrea Bolognani wrote:
This is the case-sensitive counterpart of the existing virStringHasCaseSuffix() function.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/util/virstring.c | 13 +++++++++++++ src/util/virstring.h | 2 ++ 2 files changed, 15 insertions(+)
diff --git a/src/util/virstring.c b/src/util/virstring.c index b4d10f9884..b9b33e3d1a 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1235,6 +1235,19 @@ virStringReplace(const char *haystack, return virBufferContentAndReset(&buf); }
+int
Why doesn't this return bool?
+virStringHasSuffix(const char *str, + const char *suffix)
ACK with appropriate return type.

When dealing with internal paths we don't need to worry about whether or not suffixes are lowercase since we have full control over them, which means we can avoid performing case-insensitive string comparisons. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/virsecretobj.c | 2 +- src/conf/virstorageobj.c | 2 +- src/libvirt_private.syms | 1 + src/vmx/vmx.c | 2 +- tests/testutils.c | 4 ++-- tests/virschematest.c | 4 ++-- tools/nss/libvirt_nss.c | 4 ++-- 7 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index b01dc7cc70..7800912bff 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -949,7 +949,7 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets, char *path; virSecretObjPtr obj; - if (!virStringHasCaseSuffix(de->d_name, ".xml")) + if (!virStringHasSuffix(de->d_name, ".xml")) continue; if (!(path = virFileBuildPath(configDir, de->d_name, NULL))) diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index a30d86d070..1d6c9d1937 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1719,7 +1719,7 @@ virStoragePoolObjLoadAllConfigs(virStoragePoolObjListPtr pools, char *autostartLink; virStoragePoolObjPtr obj; - if (!virStringHasCaseSuffix(entry->d_name, ".xml")) + if (!virStringHasSuffix(entry->d_name, ".xml")) continue; if (!(path = virFileBuildPath(configDir, entry->d_name, NULL))) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d7ccd509e9..1eab68622d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2958,6 +2958,7 @@ virStringFilterChars; virStringHasCaseSuffix; virStringHasChars; virStringHasControlChars; +virStringHasSuffix; virStringIsEmpty; virStringIsPrintable; virStringListAdd; diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index bbf8e55c3f..8ffd5ff088 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1600,7 +1600,7 @@ virVMXParseConfig(virVMXContext *ctx, if (virVMXGetConfigString(conf, "guestOS", &guestOS, true) < 0) goto cleanup; - if (guestOS != NULL && virStringHasCaseSuffix(guestOS, "-64")) { + if (guestOS != NULL && virStringHasSuffix(guestOS, "-64")) { def->os.arch = VIR_ARCH_X86_64; } else { def->os.arch = VIR_ARCH_I686; diff --git a/tests/testutils.c b/tests/testutils.c index cfdf122ae0..d2aa4e5d49 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -525,8 +525,8 @@ virTestRewrapFile(const char *filename) char *script = NULL; virCommandPtr cmd = NULL; - if (!(virStringHasCaseSuffix(filename, ".args") || - virStringHasCaseSuffix(filename, ".ldargs"))) + if (!(virStringHasSuffix(filename, ".args") || + virStringHasSuffix(filename, ".ldargs"))) return 0; if (!perl) { diff --git a/tests/virschematest.c b/tests/virschematest.c index 46c67760a7..4e4f912699 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -41,7 +41,7 @@ static int testSchemaFile(const void *args) { const struct testSchemaData *data = args; - bool shouldFail = virStringHasCaseSuffix(data->xml_path, "-invalid.xml"); + bool shouldFail = virStringHasSuffix(data->xml_path, "-invalid.xml"); xmlDocPtr xml = NULL; int ret = -1; @@ -82,7 +82,7 @@ testSchemaDir(const char *schema, return -1; while ((rc = virDirRead(dir, &ent, dir_path)) > 0) { - if (!virStringHasCaseSuffix(ent->d_name, ".xml")) + if (!virStringHasSuffix(ent->d_name, ".xml")) continue; if (ent->d_name[0] == '.') continue; diff --git a/tools/nss/libvirt_nss.c b/tools/nss/libvirt_nss.c index 4d86f8e6ce..3ff1bada31 100644 --- a/tools/nss/libvirt_nss.c +++ b/tools/nss/libvirt_nss.c @@ -274,7 +274,7 @@ findLease(const char *name, while ((ret = virDirRead(dir, &entry, leaseDir)) > 0) { char *path; - if (virStringHasCaseSuffix(entry->d_name, ".status")) { + if (virStringHasSuffix(entry->d_name, ".status")) { if (!(path = virFileBuildPath(leaseDir, entry->d_name, NULL))) goto cleanup; @@ -285,7 +285,7 @@ findLease(const char *name, goto cleanup; } VIR_FREE(path); - } else if (virStringHasCaseSuffix(entry->d_name, ".macs")) { + } else if (virStringHasSuffix(entry->d_name, ".macs")) { if (!(path = virFileBuildPath(leaseDir, entry->d_name, NULL))) goto cleanup; -- 2.20.1

On Wed, Mar 06, 2019 at 17:54:04 +0100, Andrea Bolognani wrote:
When dealing with internal paths we don't need to worry about whether or not suffixes are lowercase since we have full control over them, which means we can avoid performing case-insensitive string comparisons.
Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/conf/virsecretobj.c | 2 +- src/conf/virstorageobj.c | 2 +- src/libvirt_private.syms | 1 + src/vmx/vmx.c | 2 +- tests/testutils.c | 4 ++-- tests/virschematest.c | 4 ++-- tools/nss/libvirt_nss.c | 4 ++-- 7 files changed, 10 insertions(+), 9 deletions(-)
[...]
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d7ccd509e9..1eab68622d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2958,6 +2958,7 @@ virStringFilterChars; virStringHasCaseSuffix; virStringHasChars; virStringHasControlChars; +virStringHasSuffix;
This should be in previous commit.
virStringIsEmpty; virStringIsPrintable; virStringListAdd;
ACK with that fixed.
participants (2)
-
Andrea Bolognani
-
Peter Krempa