The function returns how many array items were filled in, but virsh
never checked for anything other than errors.  Just to make sure this
does not report invalid data, even though the only possibility would be
reporting 0 free pages, check the returned data so that possible errors
are detected.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 tools/virsh-host.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index 21aca5f6dc83..411648197895 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -328,6 +328,8 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
     bool cellno = vshCommandOptBool(cmd, "cellno");
     bool pagesz = vshCommandOptBool(cmd, "pagesize");
     virshControl *priv = ctl->privData;
+    bool pagesize_missing = false;
+    int rv = -1;
 
     VSH_EXCLUSIVE_OPTIONS_VAR(all, cellno);
 
@@ -407,16 +409,22 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
                 goto cleanup;
             }
 
-            if (virNodeGetFreePages(priv->conn, npages, pagesize,
-                                    cell, 1, counts, 0) < 0)
+            rv = virNodeGetFreePages(priv->conn, npages, pagesize,
+                                     cell, 1, counts, 0);
+            if (rv < 0)
                 goto cleanup;
 
+            if (rv < npages) {
+                pagesize_missing = true;
+                vshError(ctl, _("Did not get all free page data for node %1$d"), cell);
+                continue;
+            }
+
             vshPrint(ctl, _("Node %1$d:\n"), cell);
             for (j = 0; j < npages; j++)
                 vshPrint(ctl, "%uKiB: %lld\n", pagesize[j], counts[j]);
             vshPrint(ctl, "%c", '\n');
         }
-
     } else {
         if (!cellno) {
             vshError(ctl, "%s", _("missing cellno argument"));
@@ -443,14 +451,22 @@ cmdFreepages(vshControl *ctl, const vshCmd *cmd)
 
         counts = g_new0(unsigned long long, 1);
 
-        if (virNodeGetFreePages(priv->conn, 1, pagesize,
-                                cell, 1, counts, 0) < 0)
+        rv = virNodeGetFreePages(priv->conn, 1, pagesize,
+                                 cell, 1, counts, 0);
+        if (rv < 0)
             goto cleanup;
 
+        if (rv == 0) {
+            vshError(ctl,
+                     "Could not get count of free %uKiB pages, no data returned",
+                     *pagesize);
+            goto cleanup;
+        }
+
         vshPrint(ctl, "%uKiB: %lld\n", *pagesize, counts[0]);
     }
 
-    ret = true;
+    ret = !pagesize_missing;
  cleanup:
     VIR_FREE(nodes);
     return ret;
-- 
2.42.0

On 9/26/23 15:56, Martin Kletzander wrote:
> The function returns how many array items were filled in, but virsh
> never checked for anything other than errors.  Just to make sure this
> does not report invalid data, even though the only possibility would be
> reporting 0 free pages, check the returned data so that possible errors
> are detected.
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  tools/virsh-host.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)

Yeah. This should never happen though (at least in real life
conditions), because our virHostMemGetFreePages() either fills
everything or returns an error. But the way our public API is documented
warrants having this in.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal