diff mbox

[v2,2/2] x86/mm: Annotate gfn_get_* helpers as requiring non-NULL parameters

Message ID 1470070776-19018-1-git-send-email-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Cooper Aug. 1, 2016, 4:59 p.m. UTC
Introduce and use the nonnull attribute to help the compiler catch NULL
parameters being passed to function which require their parameters not to be
NULL.  Experimentally, GCC 4.9 on Debian Jessie only warns of non-NULL-ness
from immediate callers, so propagate the attributes out to all helpers.

A sample error looks like:

mem_sharing.c: In function ‘mem_sharing_nominate_page’:
mem_sharing.c:884:13: error: null argument where non-null required (argument 3) [-Werror=nonnull]
             amfn = get_gfn_type_access(ap2m, gfn, NULL, &ap2ma, 0, NULL);
             ^

As part of this, replace the get_gfn_type_access() macro with an equivalent
static inline function for extra type safety, and the ability to be annotated.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: George Dunlap <george.dunlap@eu.citrix.com>
CC: Tamas K Lengyel <tamas.lengyel@zentific.com>

v2:
 * s/nonnull/__nonnull__/
 * Tolerate the p2m parameter being NULL
---
 xen/include/asm-x86/p2m.h  | 19 +++++++++++--------
 xen/include/xen/compiler.h |  1 +
 2 files changed, 12 insertions(+), 8 deletions(-)

Comments

Jan Beulich Aug. 2, 2016, 7:18 a.m. UTC | #1
>>> On 01.08.16 at 18:59, <andrew.cooper3@citrix.com> wrote:
> Introduce and use the nonnull attribute to help the compiler catch NULL
> parameters being passed to function which require their parameters not to be
> NULL.  Experimentally, GCC 4.9 on Debian Jessie only warns of non-NULL-ness
> from immediate callers, so propagate the attributes out to all helpers.
> 
> A sample error looks like:
> 
> mem_sharing.c: In function ‘mem_sharing_nominate_page’:
> mem_sharing.c:884:13: error: null argument where non-null required (argument 
> 3) [-Werror=nonnull]
>              amfn = get_gfn_type_access(ap2m, gfn, NULL, &ap2ma, 0, NULL);
>              ^
> 
> As part of this, replace the get_gfn_type_access() macro with an equivalent
> static inline function for extra type safety, and the ability to be 
> annotated.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
George Dunlap Aug. 2, 2016, 1:14 p.m. UTC | #2
On Mon, Aug 1, 2016 at 5:59 PM, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> Introduce and use the nonnull attribute to help the compiler catch NULL
> parameters being passed to function which require their parameters not to be
> NULL.  Experimentally, GCC 4.9 on Debian Jessie only warns of non-NULL-ness
> from immediate callers, so propagate the attributes out to all helpers.
>
> A sample error looks like:
>
> mem_sharing.c: In function ‘mem_sharing_nominate_page’:
> mem_sharing.c:884:13: error: null argument where non-null required (argument 3) [-Werror=nonnull]
>              amfn = get_gfn_type_access(ap2m, gfn, NULL, &ap2ma, 0, NULL);
>              ^
>
> As part of this, replace the get_gfn_type_access() macro with an equivalent
> static inline function for extra type safety, and the ability to be annotated.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: George Dunlap <george.dunlap@citrix.com>
diff mbox

Patch

diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h
index 194020e..035ca92 100644
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -380,9 +380,9 @@  void p2m_unlock_and_tlb_flush(struct p2m_domain *p2m);
  * After calling any of the variants below, caller needs to use
  * put_gfn. ****/
 
-mfn_t __get_gfn_type_access(struct p2m_domain *p2m, unsigned long gfn,
-                    p2m_type_t *t, p2m_access_t *a, p2m_query_t q,
-                    unsigned int *page_order, bool_t locked);
+mfn_t __nonnull(3, 4) __get_gfn_type_access(
+    struct p2m_domain *p2m, unsigned long gfn, p2m_type_t *t,
+    p2m_access_t *a, p2m_query_t q, unsigned int *page_order, bool_t locked);
 
 /* Read a particular P2M table, mapping pages as we go.  Most callers
  * should _not_ call this directly; use the other get_gfn* functions
@@ -391,13 +391,16 @@  mfn_t __get_gfn_type_access(struct p2m_domain *p2m, unsigned long gfn,
  * If the lookup succeeds, the return value is != INVALID_MFN and 
  * *page_order is filled in with the order of the superpage (if any) that
  * the entry was found in.  */
-#define get_gfn_type_access(p, g, t, a, q, o)   \
-        __get_gfn_type_access((p), (g), (t), (a), (q), (o), 1)
+static inline mfn_t __nonnull(3, 4) get_gfn_type_access(
+    struct p2m_domain *p2m, unsigned long gfn, p2m_type_t *t,
+    p2m_access_t *a, p2m_query_t q, unsigned int *page_order)
+{
+    return __get_gfn_type_access(p2m, gfn, t, a, q, page_order, true);
+}
 
 /* General conversion function from gfn to mfn */
-static inline mfn_t get_gfn_type(struct domain *d,
-                                    unsigned long gfn, p2m_type_t *t,
-                                    p2m_query_t q)
+static inline mfn_t __nonnull(3) get_gfn_type(
+    struct domain *d, unsigned long gfn, p2m_type_t *t, p2m_query_t q)
 {
     p2m_access_t a;
     return get_gfn_type_access(p2m_get_hostp2m(d), gfn, t, &a, q, NULL);
diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index 892455b..f3e8d95 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -61,6 +61,7 @@ 
 #define __maybe_unused __attribute__((__unused__))
 
 #define __must_check __attribute__((__warn_unused_result__))
+#define __nonnull(...) __attribute__((__nonnull__(__VA_ARGS__)))
 
 #define offsetof(a,b) __builtin_offsetof(a,b)