While glibc provides qsort(), which usually is just a mergesort, until sorting arrays so huge that temporary array used by mergesort would not fit into physical memory (which in our case is never), we are not guaranteed it'll use mergesort. The advantage of mergesort is clear - it's stable. IOW, if we have an array of values parsed from XML, qsort() it and produce some output based on those values, we can then compare the output with some expected output, line by line. But with newer glibc this is all history. After [1], qsort() is no longer mergesort but introsort instead, which is not stable. This is suboptimal, because in some cases we want to preserve order of equal items. For instance, in ebiptablesApplyNewRules(), nwfilter rules are sorted by their priority. But if two rules have the same priority, we want to keep them in the order they appear in the XML. Since it's hard/needless work to identify places where stable or unstable sorting is needed, let's just play it safe and use stable sorting everywhere. Fortunately, glib provides g_qsort_with_data() which indeed implement mergesort and it's a drop in replacement for qsort(), almost. It accepts fifth argument (pointer to opaque data), that is passed to comparator function, which then accepts three arguments. We have to keep one occurance of qsort() though - in NSS module which deliberately does not link with glib. 1: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=03bf8357e8291857a4... Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- build-aux/Makefile.nonreentrant | 3 +-- build-aux/syntax-check.mk | 2 +- src/conf/capabilities.c | 8 ++++--- src/conf/domain_conf.c | 6 +++-- src/cpu/cpu.c | 7 +++--- src/cpu/cpu_x86.c | 15 +++++++----- src/lxc/lxc_container.c | 3 ++- src/nwfilter/nwfilter_ebiptables_driver.c | 29 +++++++++++++++-------- src/qemu/qemu_monitor_json.c | 6 +++-- src/qemu/qemu_process.c | 7 +++--- src/security/security_manager.c | 9 ++++--- src/util/virfile.c | 8 ++++--- src/util/virhash.c | 9 ++++--- src/util/virresctrl.c | 9 ++++--- src/util/virstring.c | 12 ++++++---- src/util/virstring.h | 8 +++++-- src/util/virtypedparam.c | 9 ++++--- tests/commandhelper.c | 6 +++-- tests/virstringtest.c | 8 +++---- tools/virsh-checkpoint.c | 10 ++++---- tools/virsh-domain-monitor.c | 11 +++++---- tools/virsh-host.c | 7 ++++-- tools/virsh-interface.c | 11 +++++---- tools/virsh-network.c | 29 +++++++++++++++-------- tools/virsh-nodedev.c | 11 +++++---- tools/virsh-nwfilter.c | 22 ++++++++++------- tools/virsh-pool.c | 11 +++++---- tools/virsh-secret.c | 11 +++++---- tools/virsh-snapshot.c | 10 ++++---- tools/virsh-volume.c | 10 +++++--- 30 files changed, 196 insertions(+), 111 deletions(-) diff --git a/build-aux/Makefile.nonreentrant b/build-aux/Makefile.nonreentrant index 729f395e9d..4ecd694b2f 100644 --- a/build-aux/Makefile.nonreentrant +++ b/build-aux/Makefile.nonreentrant @@ -20,11 +20,10 @@ # nm -D --defined-only --without-symbol-versions /lib64/libc.so.6 \ # | grep '_r$' \ # | awk '{print $3}' \ -# | grep -v -e '^_' -e 'qsort' -e 'readdir' \ +# | grep -v -e '^_' -e 'readdir' \ # | sort -u \ # | sed -s 's/_r$//' # -# qsort() is safe because we don't need to pass extra args to qsort comparator, # readdir*() is safe as long as each DIR * instance is only used by one thread. # # Also manually add in all inet_* functions some of which diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 53dcf11a06..309ca0c29a 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -380,7 +380,7 @@ sc_prohibit_unsigned_pid: # Many of the function names below came from this filter: # git grep -B2 '\<_('|grep -E '\.c- *[[:alpha:]_][[:alnum:]_]* ?\(.*[,;]$' \ # |sed 's/.*\.c- *//'|perl -pe 's/ ?\(.*//'|sort -u \ -# |grep -vE '^(qsort|if|close|assert|fputc|free|N_|vir.*GetName|.*Unlock|virNodeListDevices|virHashRemoveEntry|freeaddrinfo|.*[fF]ree|xdrmem_create|xmlXPathFreeObject|virUUIDFormat|openvzSetProgramSentinal|polkit_action_unref)$' +# |grep -vE '^(if|close|assert|fputc|free|N_|vir.*GetName|.*Unlock|virNodeListDevices|virHashRemoveEntry|freeaddrinfo|.*[fF]ree|xdrmem_create|xmlXPathFreeObject|virUUIDFormat|openvzSetProgramSentinal|polkit_action_unref)$' msg_gen_function = msg_gen_function += VIR_ERROR diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 34f04cb7d3..32badee7b3 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -2073,7 +2073,8 @@ virCapsHostCacheBankFree(virCapsHostCacheBank *ptr) static int virCapsHostCacheBankSorter(const void *a, - const void *b) + const void *b, + void *opaque G_GNUC_UNUSED) { virCapsHostCacheBank *ca = *(virCapsHostCacheBank **)a; virCapsHostCacheBank *cb = *(virCapsHostCacheBank **)b; @@ -2273,8 +2274,9 @@ virCapabilitiesInitCaches(virCaps *caps) * still traverse the directory instead of guessing names (in case there is * 'index1' and 'index3' but no 'index2'). */ if (caps->host.cache.banks) { - qsort(caps->host.cache.banks, caps->host.cache.nbanks, - sizeof(*caps->host.cache.banks), virCapsHostCacheBankSorter); + g_qsort_with_data(caps->host.cache.banks, caps->host.cache.nbanks, + sizeof(*caps->host.cache.banks), + virCapsHostCacheBankSorter, NULL); } if (virCapabilitiesInitResctrlMemory(caps) < 0) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 27f6862067..f7d7244738 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -15703,7 +15703,9 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt, } -static int virDomainIdMapEntrySort(const void *a, const void *b) +static int virDomainIdMapEntrySort(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { const virDomainIdMapEntry *entrya = a; const virDomainIdMapEntry *entryb = b; @@ -15746,7 +15748,7 @@ virDomainIdmapDefParseXML(xmlXPathContextPtr ctxt, } } - qsort(idmap, num, sizeof(idmap[0]), virDomainIdMapEntrySort); + g_qsort_with_data(idmap, num, sizeof(idmap[0]), virDomainIdMapEntrySort, NULL); return idmap; } diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index bb5737e938..bc43aa4e93 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -1045,7 +1045,8 @@ virCPUConvertLegacy(virArch arch, static int virCPUFeatureCompare(const void *p1, - const void *p2) + const void *p2, + void *opaque G_GNUC_UNUSED) { const virCPUFeatureDef *f1 = p1; const virCPUFeatureDef *f2 = p2; @@ -1085,8 +1086,8 @@ virCPUExpandFeatures(virArch arch, driver->expandFeatures(cpu) < 0) return -1; - qsort(cpu->features, cpu->nfeatures, sizeof(*cpu->features), - virCPUFeatureCompare); + g_qsort_with_data(cpu->features, cpu->nfeatures, sizeof(*cpu->features), + virCPUFeatureCompare, NULL); VIR_DEBUG("nfeatures=%zu", cpu->nfeatures); return 0; diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index 7a7f3b409d..8d0e3947ce 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -393,7 +393,9 @@ x86FeatureFindInternal(const char *name) static int -virCPUx86DataSorter(const void *a, const void *b) +virCPUx86DataSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virCPUx86DataItem *da = (virCPUx86DataItem *) a; virCPUx86DataItem *db = (virCPUx86DataItem *) b; @@ -437,7 +439,7 @@ static int virCPUx86DataItemCmp(const virCPUx86DataItem *item1, const virCPUx86DataItem *item2) { - return virCPUx86DataSorter(item1, item2); + return virCPUx86DataSorter(item1, item2, NULL); } @@ -541,8 +543,9 @@ virCPUx86DataAddItem(virCPUx86Data *data, VIR_APPEND_ELEMENT_COPY(data->items, data->len, *((virCPUx86DataItem *)item)); - qsort(data->items, data->len, - sizeof(virCPUx86DataItem), virCPUx86DataSorter); + g_qsort_with_data(data->items, data->len, + sizeof(virCPUx86DataItem), + virCPUx86DataSorter, NULL); } return 0; @@ -3465,8 +3468,8 @@ virCPUx86DataGetHost(void) } /* the rest of the code expects the function to be in order */ - qsort(cpuid->data.x86.items, cpuid->data.x86.len, - sizeof(virCPUx86DataItem), virCPUx86DataSorter); + g_qsort_with_data(cpuid->data.x86.items, cpuid->data.x86.len, + sizeof(virCPUx86DataItem), virCPUx86DataSorter, NULL); return cpuid; } diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 35a882171b..652697890f 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -791,7 +791,8 @@ static int lxcContainerSetReadOnly(void) if (!mounts) return 0; - qsort(mounts, nmounts, sizeof(mounts[0]), virStringSortRevCompare); + g_qsort_with_data(mounts, nmounts, + sizeof(mounts[0]), virStringSortRevCompare, NULL); /* turn 'mounts' into a proper GStrv */ VIR_EXPAND_N(mounts, nmounts, 1); diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 1c5da2ae05..56bddb9097 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -3087,7 +3087,9 @@ virNWFilterRuleInstSort(const void *a, const void *b) static int -virNWFilterRuleInstSortPtr(const void *a, const void *b) +virNWFilterRuleInstSortPtr(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virNWFilterRuleInst * const *insta = a; virNWFilterRuleInst * const *instb = b; @@ -3097,7 +3099,8 @@ virNWFilterRuleInstSortPtr(const void *a, const void *b) static int ebiptablesFilterOrderSort(const void *va, - const void *vb) + const void *vb, + void *opaque G_GNUC_UNUSED) { const virHashKeyValuePair *a = va; const virHashKeyValuePair *b = vb; @@ -3244,7 +3247,9 @@ struct _ebtablesSubChainInst { static int -ebtablesSubChainInstSort(const void *a, const void *b) +ebtablesSubChainInstSort(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { const ebtablesSubChainInst **insta = (const ebtablesSubChainInst **)a; const ebtablesSubChainInst **instb = (const ebtablesSubChainInst **)b; @@ -3268,7 +3273,8 @@ ebtablesGetSubChainInsts(GHashTable *chains, if (filter_names == NULL) return -1; - qsort(filter_names, nfilter_names, sizeof(*filter_names), ebiptablesFilterOrderSort); + g_qsort_with_data(filter_names, nfilter_names, + sizeof(*filter_names), ebiptablesFilterOrderSort, NULL); for (i = 0; filter_names[i].key; i++) { g_autofree ebtablesSubChainInst *inst = NULL; @@ -3306,9 +3312,10 @@ ebiptablesApplyNewRules(const char *ifname, size_t nsubchains = 0; int ret = -1; - if (nrules) - qsort(rules, nrules, sizeof(rules[0]), - virNWFilterRuleInstSortPtr); + if (nrules) { + g_qsort_with_data(rules, nrules, sizeof(rules[0]), + virNWFilterRuleInstSortPtr, NULL); + } /* cleanup whatever may exist */ virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS); @@ -3388,9 +3395,11 @@ ebiptablesApplyNewRules(const char *ifname, goto cleanup; } - if (nsubchains > 0) - qsort(subchains, nsubchains, sizeof(subchains[0]), - ebtablesSubChainInstSort); + if (nsubchains > 0) { + g_qsort_with_data(subchains, nsubchains, + sizeof(subchains[0]), + ebtablesSubChainInstSort, NULL); + } for (i = 0, j = 0; i < nrules; i++) { if (virNWFilterRuleIsProtocolEthernet(rules[i]->def)) { diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 105d729d7c..6f1dffe8f7 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7607,7 +7607,8 @@ qemuMonitorJSONProcessHotpluggableCpusReply(virJSONValue *vcpu, static int qemuMonitorQueryHotpluggableCpusEntrySort(const void *p1, - const void *p2) + const void *p2, + void *opaque G_GNUC_UNUSED) { const struct qemuMonitorQueryHotpluggableCpusEntry *a = p1; const struct qemuMonitorQueryHotpluggableCpusEntry *b = p2; @@ -7659,7 +7660,8 @@ qemuMonitorJSONGetHotpluggableCPUs(qemuMonitor *mon, goto cleanup; } - qsort(info, ninfo, sizeof(*info), qemuMonitorQueryHotpluggableCpusEntrySort); + g_qsort_with_data(info, ninfo, sizeof(*info), + qemuMonitorQueryHotpluggableCpusEntrySort, NULL); *entries = g_steal_pointer(&info); *nentries = ninfo; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index b9267d8699..f32e82bbd1 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6093,7 +6093,8 @@ qemuDomainHasHotpluggableStartupVcpus(virDomainDef *def) static int qemuProcessVcpusSortOrder(const void *a, - const void *b) + const void *b, + void *opaque G_GNUC_UNUSED) { virDomainVcpuDef *vcpua = *((virDomainVcpuDef **)a); virDomainVcpuDef *vcpub = *((virDomainVcpuDef **)b); @@ -6133,8 +6134,8 @@ qemuProcessSetupHotpluggableVcpus(virDomainObj *vm, if (nbootHotplug == 0) return 0; - qsort(bootHotplug, nbootHotplug, sizeof(*bootHotplug), - qemuProcessVcpusSortOrder); + g_qsort_with_data(bootHotplug, nbootHotplug, + sizeof(*bootHotplug), qemuProcessVcpusSortOrder, NULL); if (virDomainCgroupEmulatorAllNodesAllow(priv->cgroup, &emulatorCgroup) < 0) goto cleanup; diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 5df0cd3419..afd41f1c20 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -1247,7 +1247,9 @@ virSecurityManagerRestoreNetdevLabel(virSecurityManager *mgr, static int -cmpstringp(const void *p1, const void *p2) +cmpstringp(const void *p1, + const void *p2, + void *opaque G_GNUC_UNUSED) { const char *s1 = *(char * const *) p1; const char *s2 = *(char * const *) p2; @@ -1303,8 +1305,9 @@ virSecurityManagerMetadataLock(virSecurityManager *mgr G_GNUC_UNUSED, * paths in the same order and thus no deadlock can occur. * Lastly, it makes searching for duplicate paths below * simpler. */ - if (paths) - qsort(paths, npaths, sizeof(*paths), cmpstringp); + if (paths) { + g_qsort_with_data(paths, npaths, sizeof(*paths), cmpstringp, NULL); + } for (i = 0; i < npaths; i++) { const char *p = paths[i]; diff --git a/src/util/virfile.c b/src/util/virfile.c index 54708652fb..007b6cf512 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2193,9 +2193,11 @@ virFileGetMountSubtreeImpl(const char *mtabpath, mounts[nmounts - 2] = g_strdup(mntent.mnt_dir); } - if (mounts) - qsort(mounts, nmounts - 1, sizeof(mounts[0]), - reverse ? virStringSortRevCompare : virStringSortCompare); + if (mounts) { + g_qsort_with_data(mounts, nmounts - 1, sizeof(mounts[0]), + reverse ? virStringSortRevCompare : virStringSortCompare, + NULL); + } *mountsret = mounts; *nmountsret = nmounts ? nmounts - 1 : 0; diff --git a/src/util/virhash.c b/src/util/virhash.c index 8365f51eb3..19908c9412 100644 --- a/src/util/virhash.c +++ b/src/util/virhash.c @@ -514,7 +514,8 @@ void *virHashSearch(GHashTable *table, static int virHashGetItemsKeySorter(const void *va, - const void *vb) + const void *vb, + void *opaque G_GNUC_UNUSED) { const virHashKeyValuePair *a = va; const virHashKeyValuePair *b = vb; @@ -552,8 +553,10 @@ virHashGetItems(GHashTable *table, i++; } - if (sortKeys) - qsort(items, *nitems, sizeof(*items), virHashGetItemsKeySorter); + if (sortKeys) { + g_qsort_with_data(items, *nitems, + sizeof(*items), virHashGetItemsKeySorter, NULL); + } return items; } diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 8ca58f5d10..30ae25c487 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -2522,7 +2522,8 @@ virResctrlMonitorRemove(virResctrlMonitor *monitor) static int virResctrlMonitorStatsSorter(const void *a, - const void *b) + const void *b, + void *opaque G_GNUC_UNUSED) { return (*(virResctrlMonitorStats **)a)->id - (*(virResctrlMonitorStats **)b)->id; @@ -2625,8 +2626,10 @@ virResctrlMonitorGetStats(virResctrlMonitor *monitor, } /* Sort in id's ascending order */ - if (*nstats) - qsort(*stats, *nstats, sizeof(**stats), virResctrlMonitorStatsSorter); + if (*nstats) { + g_qsort_with_data(*stats, *nstats, sizeof(**stats), + virResctrlMonitorStatsSorter, NULL); + } ret = 0; cleanup: diff --git a/src/util/virstring.c b/src/util/virstring.c index 6b728ff047..81c9aff304 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -518,9 +518,11 @@ virStringIsEmpty(const char *str) * virStringSortCompare: * * A comparator function for sorting strings in - * normal order with qsort(). + * normal order with g_qsort_with_data(). */ -int virStringSortCompare(const void *a, const void *b) +int virStringSortCompare(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { const char **sa = (const char**)a; const char **sb = (const char**)b; @@ -532,9 +534,11 @@ int virStringSortCompare(const void *a, const void *b) * virStringSortRevCompare: * * A comparator function for sorting strings in - * reverse order with qsort(). + * reverse order with g_qsort_with_data(). */ -int virStringSortRevCompare(const void *a, const void *b) +int virStringSortRevCompare(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { const char **sa = (const char**)a; const char **sb = (const char**)b; diff --git a/src/util/virstring.h b/src/util/virstring.h index 16dcce98f4..8f9b1edc8f 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -83,8 +83,12 @@ bool virStringIsEmpty(const char *str); int virStrcpy(char *dest, const char *src, size_t destbytes); #define virStrcpyStatic(dest, src) virStrcpy((dest), (src), sizeof(dest)) -int virStringSortCompare(const void *a, const void *b); -int virStringSortRevCompare(const void *a, const void *b); +int virStringSortCompare(const void *a, + const void *b, + void *opaque); +int virStringSortRevCompare(const void *a, + const void *b, + void *opaque); int virStringToUpper(char **dst, const char *src); ssize_t virStringSearch(const char *str, diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c index ef3b8052f6..1be249d855 100644 --- a/src/util/virtypedparam.c +++ b/src/util/virtypedparam.c @@ -43,7 +43,9 @@ VIR_ENUM_IMPL(virTypedParameter, ); static int -virTypedParamsSortName(const void *left, const void *right) +virTypedParamsSortName(const void *left, + const void *right, + void *opaque G_GNUC_UNUSED) { const virTypedParameter *param_left = left, *param_right = right; return strcmp(param_left->field, param_right->field); @@ -78,7 +80,8 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) /* Here we intentionally don't copy values */ memcpy(sorted, params, sizeof(*params) * nparams); - qsort(sorted, nparams, sizeof(*sorted), virTypedParamsSortName); + g_qsort_with_data(sorted, nparams, + sizeof(*sorted), virTypedParamsSortName, NULL); name = va_arg(ap, const char *); while (name) { @@ -102,7 +105,7 @@ virTypedParamsValidate(virTypedParameterPtr params, int nparams, ...) va_end(ap); - qsort(keys, nkeys, sizeof(*keys), virTypedParamsSortName); + g_qsort_with_data(keys, nkeys, sizeof(*keys), virTypedParamsSortName, NULL); for (i = 0, j = 0; i < nparams && j < nkeys;) { if (STRNEQ(sorted[i].field, keys[j].field)) { diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 334b02122c..d4629d824e 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -129,7 +129,9 @@ static void printArguments(FILE *log, int argc, char** argv) } } -static int envsort(const void *a, const void *b) +static int envsort(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { const char *astr = *(const char**)a; const char *bstr = *(const char**)b; @@ -165,7 +167,7 @@ static int printEnvironment(FILE *log) newenv[i] = environ[i]; } - qsort(newenv, length, sizeof(newenv[0]), envsort); + g_qsort_with_data(newenv, length, sizeof(newenv[0]), envsort, NULL); for (i = 0; i < length; i++) { /* Ignore the variables used to instruct the loader into diff --git a/tests/virstringtest.c b/tests/virstringtest.c index 6e697cc240..f4976890db 100644 --- a/tests/virstringtest.c +++ b/tests/virstringtest.c @@ -89,10 +89,10 @@ testStringSortCompare(const void *opaque G_GNUC_UNUSED) }; size_t i; - qsort(randlist, G_N_ELEMENTS(randlist), sizeof(randlist[0]), - virStringSortCompare); - qsort(randrlist, G_N_ELEMENTS(randrlist), sizeof(randrlist[0]), - virStringSortRevCompare); + g_qsort_with_data(randlist, G_N_ELEMENTS(randlist), + sizeof(randlist[0]), virStringSortCompare, NULL); + g_qsort_with_data(randrlist, G_N_ELEMENTS(randrlist), + sizeof(randrlist[0]), virStringSortRevCompare, NULL); for (i = 0; i < G_N_ELEMENTS(randlist); i++) { if (STRNEQ(randlist[i], sortlist[i])) { diff --git a/tools/virsh-checkpoint.c b/tools/virsh-checkpoint.c index 727de34abb..34bae34f9a 100644 --- a/tools/virsh-checkpoint.c +++ b/tools/virsh-checkpoint.c @@ -526,7 +526,8 @@ virshCheckpointListFree(struct virshCheckpointList *checkpointlist) static int virshChkSorter(const void *a, - const void *b) + const void *b, + void *opaque G_GNUC_UNUSED) { const struct virshChk *sa = a; const struct virshChk *sb = b; @@ -594,9 +595,10 @@ virshCheckpointListCollect(vshControl *ctl, } if (!(orig_flags & VIR_DOMAIN_CHECKPOINT_LIST_TOPOLOGICAL) && - checkpointlist->chks) - qsort(checkpointlist->chks, checkpointlist->nchks, - sizeof(*checkpointlist->chks), virshChkSorter); + checkpointlist->chks) { + g_qsort_with_data(checkpointlist->chks, checkpointlist->nchks, + sizeof(*checkpointlist->chks), virshChkSorter, NULL); + } ret = g_steal_pointer(&checkpointlist); diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 89fdc7a050..a2c56fc090 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1483,7 +1483,9 @@ static const vshCmdInfo info_list[] = { /* compare domains, pack NULLed ones at the end */ static int -virshDomainSorter(const void *a, const void *b) +virshDomainSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virDomainPtr *da = (virDomainPtr *) a; virDomainPtr *db = (virDomainPtr *) b; @@ -1741,9 +1743,10 @@ virshDomainListCollect(vshControl *ctl, unsigned int flags) finished: /* sort the list */ - if (list->domains && list->ndomains) - qsort(list->domains, list->ndomains, sizeof(*list->domains), - virshDomainSorter); + if (list->domains && list->ndomains) { + g_qsort_with_data(list->domains, list->ndomains, + sizeof(*list->domains), virshDomainSorter, NULL); + } /* truncate the list if filter simulation deleted entries */ if (deleted) diff --git a/tools/virsh-host.c b/tools/virsh-host.c index 4116481978..6c14be865f 100644 --- a/tools/virsh-host.c +++ b/tools/virsh-host.c @@ -300,7 +300,9 @@ static const vshCmdOptDef opts_freepages[] = { }; static int -vshPageSizeSorter(const void *a, const void *b) +vshPageSizeSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { unsigned int pa = *(unsigned int *)a; unsigned int pb = *(unsigned int *)b; @@ -377,7 +379,8 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd) * @pagesize array will contain duplicates. We should * remove them otherwise not very nice output will be * produced. */ - qsort(pagesize, nodes_cnt, sizeof(*pagesize), vshPageSizeSorter); + g_qsort_with_data(pagesize, nodes_cnt, + sizeof(*pagesize), vshPageSizeSorter, NULL); for (i = 0; i < nodes_cnt - 1;) { if (pagesize[i] == pagesize[i + 1]) { diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 77c0fff847..09a3668438 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -141,7 +141,9 @@ cmdInterfaceEdit(vshControl *ctl, const vshCmd *cmd) } static int -virshInterfaceSorter(const void *a, const void *b) +virshInterfaceSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virInterfacePtr *ia = (virInterfacePtr *) a; virInterfacePtr *ib = (virInterfacePtr *) b; @@ -281,9 +283,10 @@ virshInterfaceListCollect(vshControl *ctl, finished: /* sort the list */ - if (list->ifaces && list->nifaces) - qsort(list->ifaces, list->nifaces, - sizeof(*list->ifaces), virshInterfaceSorter); + if (list->ifaces && list->nifaces) { + g_qsort_with_data(list->ifaces, list->nifaces, + sizeof(*list->ifaces), virshInterfaceSorter, NULL); + } /* truncate the list if filter simulation deleted entries */ if (deleted) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 998e7e15e3..68c7543863 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -791,7 +791,9 @@ cmdNetworkInfo(vshControl *ctl, const vshCmd *cmd) } static int -virshNetworkSorter(const void *a, const void *b) +virshNetworkSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virNetworkPtr *na = (virNetworkPtr *) a; virNetworkPtr *nb = (virNetworkPtr *) b; @@ -982,9 +984,10 @@ virshNetworkListCollect(vshControl *ctl, finished: /* sort the list */ - if (list->nets && list->nnets) - qsort(list->nets, list->nnets, - sizeof(*list->nets), virshNetworkSorter); + if (list->nets && list->nnets) { + g_qsort_with_data(list->nets, list->nnets, + sizeof(*list->nets), virshNetworkSorter, NULL); + } /* truncate the list if filter simulation deleted entries */ if (deleted) @@ -1801,7 +1804,9 @@ static const vshCmdOptDef opts_network_dhcp_leases[] = { }; static int -virshNetworkDHCPLeaseSorter(const void *a, const void *b) +virshNetworkDHCPLeaseSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virNetworkDHCPLeasePtr *lease1 = (virNetworkDHCPLeasePtr *) a; virNetworkDHCPLeasePtr *lease2 = (virNetworkDHCPLeasePtr *) b; @@ -1840,7 +1845,8 @@ cmdNetworkDHCPLeases(vshControl *ctl, const vshCmd *cmd) } /* Sort the list according to MAC Address/IAID */ - qsort(leases, nleases, sizeof(*leases), virshNetworkDHCPLeaseSorter); + g_qsort_with_data(leases, nleases, + sizeof(*leases), virshNetworkDHCPLeaseSorter, NULL); table = vshTableNew(_("Expiry Time"), _("MAC address"), _("Protocol"), _("IP address"), _("Hostname"), _("Client ID or DUID"), @@ -2068,7 +2074,9 @@ cmdNetworkPortDelete(vshControl *ctl, const vshCmd *cmd) static int -virshNetworkPortSorter(const void *a, const void *b) +virshNetworkPortSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virNetworkPortPtr *na = (virNetworkPortPtr *) a; virNetworkPortPtr *nb = (virNetworkPortPtr *) b; @@ -2129,9 +2137,10 @@ virshNetworkPortListCollect(vshControl *ctl, list->nports = ret; /* sort the list */ - if (list->ports && list->nports) - qsort(list->ports, list->nports, - sizeof(*list->ports), virshNetworkPortSorter); + if (list->ports && list->nports) { + g_qsort_with_data(list->ports, list->nports, + sizeof(*list->ports), virshNetworkPortSorter, NULL); + } success = true; diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 82b8fb44fc..fb38fd7fcc 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -183,7 +183,9 @@ virshNodeListLookup(int devid, bool parent, void *opaque) } static int -virshNodeDeviceSorter(const void *a, const void *b) +virshNodeDeviceSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virNodeDevicePtr *na = (virNodeDevicePtr *) a; virNodeDevicePtr *nb = (virNodeDevicePtr *) b; @@ -334,9 +336,10 @@ virshNodeDeviceListCollect(vshControl *ctl, finished: /* sort the list */ - if (list->devices && list->ndevices) - qsort(list->devices, list->ndevices, - sizeof(*list->devices), virshNodeDeviceSorter); + if (list->devices && list->ndevices) { + g_qsort_with_data(list->devices, list->ndevices, + sizeof(*list->devices), virshNodeDeviceSorter, NULL); + } /* truncate the list if filter simulation deleted entries */ if (deleted) diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 92b2b7b3bc..fa52d020e4 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -220,7 +220,9 @@ cmdNWFilterDumpXML(vshControl *ctl, const vshCmd *cmd) } static int -virshNWFilterSorter(const void *a, const void *b) +virshNWFilterSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virNWFilterPtr *fa = (virNWFilterPtr *) a; virNWFilterPtr *fb = (virNWFilterPtr *) b; @@ -323,9 +325,10 @@ virshNWFilterListCollect(vshControl *ctl, finished: /* sort the list */ - if (list->filters && list->nfilters) - qsort(list->filters, list->nfilters, - sizeof(*list->filters), virshNWFilterSorter); + if (list->filters && list->nfilters) { + g_qsort_with_data(list->filters, list->nfilters, + sizeof(*list->filters), virshNWFilterSorter, NULL); + } /* truncate the list for not found filter objects */ if (deleted) @@ -644,7 +647,9 @@ cmdNWFilterBindingDumpXML(vshControl *ctl, const vshCmd *cmd) static int -virshNWFilterBindingSorter(const void *a, const void *b) +virshNWFilterBindingSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virNWFilterBindingPtr *fa = (virNWFilterBindingPtr *) a; virNWFilterBindingPtr *fb = (virNWFilterBindingPtr *) b; @@ -702,9 +707,10 @@ virshNWFilterBindingListCollect(vshControl *ctl, list->nbindings = ret; /* sort the list */ - if (list->bindings && list->nbindings > 1) - qsort(list->bindings, list->nbindings, - sizeof(*list->bindings), virshNWFilterBindingSorter); + if (list->bindings && list->nbindings > 1) { + g_qsort_with_data(list->bindings, list->nbindings, + sizeof(*list->bindings), virshNWFilterBindingSorter, NULL); + } success = true; diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 5803530d79..36f00cf643 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -814,7 +814,9 @@ cmdPoolDumpXML(vshControl *ctl, const vshCmd *cmd) } static int -virshStoragePoolSorter(const void *a, const void *b) +virshStoragePoolSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virStoragePoolPtr *pa = (virStoragePoolPtr *) a; virStoragePoolPtr *pb = (virStoragePoolPtr *) b; @@ -1005,9 +1007,10 @@ virshStoragePoolListCollect(vshControl *ctl, finished: /* sort the list */ - if (list->pools && list->npools) - qsort(list->pools, list->npools, - sizeof(*list->pools), virshStoragePoolSorter); + if (list->pools && list->npools) { + g_qsort_with_data(list->pools, list->npools, + sizeof(*list->pools), virshStoragePoolSorter, NULL); + } /* truncate the list if filter simulation deleted entries */ if (deleted) diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index 694e16c5cb..e54712ba78 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -399,7 +399,9 @@ cmdSecretUndefine(vshControl *ctl, const vshCmd *cmd) } static int -virshSecretSorter(const void *a, const void *b) +virshSecretSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virSecretPtr *sa = (virSecretPtr *) a; virSecretPtr *sb = (virSecretPtr *) b; @@ -509,9 +511,10 @@ virshSecretListCollect(vshControl *ctl, finished: /* sort the list */ - if (list->secrets && list->nsecrets) - qsort(list->secrets, list->nsecrets, - sizeof(*list->secrets), virshSecretSorter); + if (list->secrets && list->nsecrets) { + g_qsort_with_data(list->secrets, list->nsecrets, + sizeof(*list->secrets), virshSecretSorter, NULL); + } /* truncate the list for not found secret objects */ if (deleted) diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index c85258c09a..2049872322 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -982,7 +982,9 @@ virshSnapshotListFree(struct virshSnapshotList *snaplist) } static int -virshSnapSorter(const void *a, const void *b) +virshSnapSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { const struct virshSnap *sa = a; const struct virshSnap *sb = b; @@ -1232,7 +1234,7 @@ virshSnapshotListCollect(vshControl *ctl, virDomainPtr dom, * still in list. We mark known descendants by clearing * snaps[i].parents. Sorry, this is O(n^3) - hope your * hierarchy isn't huge. XXX Is it worth making O(n^2 log n) - * by using qsort and bsearch? */ + * by using g_qsort_with_data and bsearch? */ if (start_index < 0) { vshError(ctl, _("snapshot %1$s disappeared from list"), fromname); goto cleanup; @@ -1312,8 +1314,8 @@ virshSnapshotListCollect(vshControl *ctl, virDomainPtr dom, } if (!(orig_flags & VIR_DOMAIN_SNAPSHOT_LIST_TOPOLOGICAL) && snaplist->snaps && snaplist->nsnaps) { - qsort(snaplist->snaps, snaplist->nsnaps, sizeof(*snaplist->snaps), - virshSnapSorter); + g_qsort_with_data(snaplist->snaps, snaplist->nsnaps, + sizeof(*snaplist->snaps), virshSnapSorter, NULL); } snaplist->nsnaps -= deleted; diff --git a/tools/virsh-volume.c b/tools/virsh-volume.c index 9a2b21d3f3..329ca3a66c 100644 --- a/tools/virsh-volume.c +++ b/tools/virsh-volume.c @@ -1200,7 +1200,9 @@ cmdVolDumpXML(vshControl *ctl, const vshCmd *cmd) } static int -virshStorageVolSorter(const void *a, const void *b) +virshStorageVolSorter(const void *a, + const void *b, + void *opaque G_GNUC_UNUSED) { virStorageVolPtr *va = (virStorageVolPtr *) a; virStorageVolPtr *vb = (virStorageVolPtr *) b; @@ -1299,8 +1301,10 @@ virshStorageVolListCollect(vshControl *ctl, finished: /* sort the list */ - if (list->vols && list->nvols) - qsort(list->vols, list->nvols, sizeof(*list->vols), virshStorageVolSorter); + if (list->vols && list->nvols) { + g_qsort_with_data(list->vols, list->nvols, + sizeof(*list->vols), virshStorageVolSorter, NULL); + } if (deleted) VIR_SHRINK_N(list->vols, list->nvols, deleted); -- 2.41.0

On Thu, Nov 23, 2023 at 01:19:00PM +0100, Michal Privoznik wrote: Shouldn't we also prohibit qsort_r ? With that added, this series is Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com&gt;