[PATCH XEN v8 15/29] tools/libs/foreignmemory: Support err == NULL to map.

From: Ian Campbell <ian.campbell@citrix.com>
To: ian.jackson@eu.citrix.com, wei.liu2@citrix.com, xen-devel@lists.xen.org
Cc: Ian Campbell <ian.campbell@citrix.com>
Subject: [PATCH XEN v8 15/29] tools/libs/foreignmemory: Support err == NULL to map.
Date: Fri, 15 Jan 2016 13:22:54 +0000	[thread overview]
Message-ID: <1452864188-2417-16-git-send-email-ian.campbell@citrix.com> (raw)
In-Reply-To: <1452864188-2417-1-git-send-email-ian.campbell@citrix.com>

The existing xc_map_foreign_bulk-like interface encourages callers to
miss error checking for partial failure (by forgetting to scan the err
array).

Add support for passing err==NULL which behaves in a
xc_map_foreign_pages-like manner and returns a global error for any
failure.

While documenting this also clarify the overall behaviour and the
behaviour with err!=NULL.

With this the compat wrapper of xc_map_foreign_pages() can be
simplified.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
v7: Check for NULL when allocating err.

v6: New
---
 tools/libs/foreignmemory/core.c                    | 33 +++++++++++++++++++++-
 .../libs/foreignmemory/include/xenforeignmemory.h  | 24 ++++++++++++++--
 tools/libxc/xc_foreign_memory.c                    | 22 +--------------
 3 files changed, 54 insertions(+), 25 deletions(-)

diff --git a/tools/libs/foreignmemory/core.c b/tools/libs/foreignmemory/core.c
index 21dc7ee..4e0541f 100644
--- a/tools/libs/foreignmemory/core.c
+++ b/tools/libs/foreignmemory/core.c
@@ -14,6 +14,8 @@
  */
 
 #include <stdlib.h>
+#include <assert.h>
+#include <errno.h>
 
 #include "private.h"
 
@@ -64,7 +66,36 @@ void *xenforeignmemory_map(xenforeignmemory_handle *fmem,
                            uint32_t dom, int prot,
                            const xen_pfn_t *arr, int *err, size_t num)
 {
-    return osdep_xenforeignmemory_map(fmem, dom, prot, arr, err, num);
+    void *ret;
+    int *err_to_free = NULL;
+
+    if ( err == NULL )
+        err = err_to_free = malloc(num * sizeof(int));
+
+    if ( err == NULL )
+        return NULL;
+
+    ret = osdep_xenforeignmemory_map(fmem, dom, prot, arr, err, num);
+
+    if ( ret == 0 && err_to_free )
+    {
+        int i;
+
+        for ( i = 0 ; i < num ; i++ )
+        {
+            if ( err[i] )
+            {
+                errno = -err[i];
+                (void)osdep_xenforeignmemory_unmap(fmem, ret, num);
+                ret = NULL;
+                break;
+            }
+        }
+    }
+
+    free(err_to_free);
+
+    return ret;
 }
 
 int xenforeignmemory_unmap(xenforeignmemory_handle *fmem,
diff --git a/tools/libs/foreignmemory/include/xenforeignmemory.h b/tools/libs/foreignmemory/include/xenforeignmemory.h
index a6d1bdb..0ec6325 100644
--- a/tools/libs/foreignmemory/include/xenforeignmemory.h
+++ b/tools/libs/foreignmemory/include/xenforeignmemory.h
@@ -80,10 +80,28 @@ int xenforeignmemory_close(xenforeignmemory_handle *fmem);
  *
  * prot is as for mmap(2).
  *
- * Can partially succeed. When a page cannot be mapped, its respective
- * field in @err is set to the corresponding errno value.
+ * @arr is an array of @pages gfns to be mapped linearly in the local
+ * address range. @err is an (optional) output array used to report
+ * per-page errors, as errno values.
  *
- * Returns NULL if no pages can be mapped.
+ * If @err is given (is non-NULL) then the mapping may partially
+ * succeed and return a valid pointer while also using @err to
+ * indicate the success (0) or failure (errno value) of the individual
+ * pages. The global errno thread local variable is not valid in this
+ * case.
+ *
+ * If @err is not given (is NULL) then on failure to map any page any
+ * successful mappings will be undone and NULL will be returned. errno
+ * will be set to correspond to the first failure (which may not be
+ * the most critical).
+ *
+ * It is also possible to return NULL due to a complete failure,
+ * i.e. failure to even attempt the mapping, in this case the global
+ * errno will have been set and the contents of @err (if given) is
+ * invalid.
+ *
+ * Note that it is also possible to return non-NULL with the contents
+ * of @err indicating failure to map every page.
  */
 void *xenforeignmemory_map(xenforeignmemory_handle *fmem, uint32_t dom,
                            int prot, const xen_pfn_t *arr, int *err,
diff --git a/tools/libxc/xc_foreign_memory.c b/tools/libxc/xc_foreign_memory.c
index 1737c10..4b24388 100644
--- a/tools/libxc/xc_foreign_memory.c
+++ b/tools/libxc/xc_foreign_memory.c
@@ -23,32 +23,12 @@
 void *xc_map_foreign_pages(xc_interface *xch, uint32_t dom, int prot,
                            const xen_pfn_t *arr, int num)
 {
-    void *res;
-    int i, *err;
-
     if (num < 0) {
         errno = EINVAL;
         return NULL;
     }
 
-    err = malloc(num * sizeof(*err));
-    if (!err)
-        return NULL;
-
-    res = xenforeignmemory_map(xch->fmem, dom, prot, arr, err, num);
-    if (res) {
-        for (i = 0; i < num; i++) {
-            if (err[i]) {
-                errno = -err[i];
-                munmap(res, num * PAGE_SIZE);
-                res = NULL;
-                break;
-            }
-        }
-    }
-
-    free(err);
-    return res;
+    return xenforeignmemory_map(xch->fmem, dom, prot, arr, NULL, num);
 }
 
 void *xc_map_foreign_range(xc_interface *xch,
-- 
2.1.4
