[libvirt] [PATCH] util: fix a bug found in sorting cache monitor results

From: Huaqiang <huaqiang.wang@intel.com> The qsort element is a pointer of virResctrlMonitorStats, and the comparing function's arguments have a type of pointer of virResctrlMonitorStatsPtr. Signed-off-by: Huaqiang <huaqiang.wang@intel.com> --- src/util/virresctrl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index b845f36..a190977 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -2659,8 +2659,8 @@ static int virResctrlMonitorStatsSorter(const void *a, const void *b) { - return ((virResctrlMonitorStatsPtr)a)->id - - ((virResctrlMonitorStatsPtr)b)->id; + return (*(virResctrlMonitorStatsPtr *)a)->id + - (*(virResctrlMonitorStatsPtr *)b)->id; } @@ -2758,7 +2758,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, /* Sort in id's ascending order */ if (*nstats) - qsort(*stats, *nstats, sizeof(*stat), virResctrlMonitorStatsSorter); + qsort(*stats, *nstats, sizeof(stat), virResctrlMonitorStatsSorter); ret = 0; cleanup: -- 2.7.4

On 5/10/19 5:21 PM, Wang Huaqiang wrote:
From: Huaqiang <huaqiang.wang@intel.com>
The qsort element is a pointer of virResctrlMonitorStats, and the comparing function's arguments have a type of pointer of virResctrlMonitorStatsPtr.
Signed-off-by: Huaqiang <huaqiang.wang@intel.com> --- src/util/virresctrl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index b845f36..a190977 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -2659,8 +2659,8 @@ static int virResctrlMonitorStatsSorter(const void *a, const void *b) { - return ((virResctrlMonitorStatsPtr)a)->id - - ((virResctrlMonitorStatsPtr)b)->id; + return (*(virResctrlMonitorStatsPtr *)a)->id + - (*(virResctrlMonitorStatsPtr *)b)->id; }
@@ -2758,7 +2758,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor,
/* Sort in id's ascending order */ if (*nstats) - qsort(*stats, *nstats, sizeof(*stat), virResctrlMonitorStatsSorter); + qsort(*stats, *nstats, sizeof(stat), virResctrlMonitorStatsSorter);
Nice catch, but since we're touching this line, how about getting rid of @stat completely? I mean, I'd like to change it to sizeof(**stats) because that's how big one element is. Any objections? Also, I'll update the commit message a bit before pushing. Michal

On Mon, May 13, 2019 at 12:05:47PM +0200, Michal Privoznik wrote:
On 5/10/19 5:21 PM, Wang Huaqiang wrote:
From: Huaqiang <huaqiang.wang@intel.com>
The qsort element is a pointer of virResctrlMonitorStats, and the comparing function's arguments have a type of pointer of virResctrlMonitorStatsPtr.
Signed-off-by: Huaqiang <huaqiang.wang@intel.com> --- src/util/virresctrl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index b845f36..a190977 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -2659,8 +2659,8 @@ static int virResctrlMonitorStatsSorter(const void *a, const void *b) { - return ((virResctrlMonitorStatsPtr)a)->id - - ((virResctrlMonitorStatsPtr)b)->id; + return (*(virResctrlMonitorStatsPtr *)a)->id + - (*(virResctrlMonitorStatsPtr *)b)->id; } @@ -2758,7 +2758,7 @@ virResctrlMonitorGetStats(virResctrlMonitorPtr monitor, /* Sort in id's ascending order */ if (*nstats) - qsort(*stats, *nstats, sizeof(*stat), virResctrlMonitorStatsSorter); + qsort(*stats, *nstats, sizeof(stat), virResctrlMonitorStatsSorter);
Nice catch, but since we're touching this line, how about getting rid of @stat completely? I mean, I'd like to change it to sizeof(**stats) because that's how big one element is.
Yeah, I'd prefer using sizeof(**stats) as that is more obviously correct when comparing args. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (3)
-
Daniel P. Berrangé
-
Michal Privoznik
-
Wang Huaqiang