diff mbox series

[2/8] xen/public: sysctl: set_parameter.params and debug.keys should be const

Message ID 20200330192157.1335-3-julien@xen.org (mailing list archive)
State New, archived
Headers show
Series Fix build with using OCaml 4.06.1 and -safe-string | expand

Commit Message

Julien Grall March 30, 2020, 7:21 p.m. UTC 
From: Julien Grall <jgrall@amazon.com>

The fields set_parameter.params and debug.keys should never be modified
by the hypervisor. So mark them as const.

Signed-off-by: Julien Grall <jgrall@amazon.com>

---

I am not entirely sure whether we should bump the systcl version for
this change. Any thoughts?
---
 xen/include/public/sysctl.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jan Beulich March 31, 2020, 7:30 a.m. UTC | #1 
On 30.03.2020 21:21, Julien Grall wrote:
> From: Julien Grall <jgrall@amazon.com>
> 
> The fields set_parameter.params and debug.keys should never be modified
> by the hypervisor. So mark them as const.
> 
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

> I am not entirely sure whether we should bump the systcl version for
> this change. Any thoughts?

No, it should be left as is - it's about _binary_ compatibility (e.g.
if structure layout changes, or a sub-function gets dropped). The
need to potentially address build issues resulting from changes like
the one here isn't covered by it, but by the __XEN__ / __XEN_TOOLS__
conditional at the top of the header.

Jan
Julien Grall April 1, 2020, 9:53 a.m. UTC | #2 
Hi Jan,

On 31/03/2020 08:30, Jan Beulich wrote:
> On 30.03.2020 21:21, Julien Grall wrote:
>> From: Julien Grall <jgrall@amazon.com>
>>
>> The fields set_parameter.params and debug.keys should never be modified
>> by the hypervisor. So mark them as const.
>>
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
>> I am not entirely sure whether we should bump the systcl version for
>> this change. Any thoughts?
> 
> No, it should be left as is - it's about _binary_ compatibility (e.g.
> if structure layout changes, or a sub-function gets dropped). The
> need to potentially address build issues resulting from changes like
> the one here isn't covered by it, but by the __XEN__ / __XEN_TOOLS__
> conditional at the top of the header.

Thank you for the examplanation! I will commit the patch.

Cheers,
diff mbox series

Patch

diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index 3d72fab49f..3a08c512e8 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -170,7 +170,7 @@  struct xen_sysctl_getdomaininfolist {
 /* XEN_SYSCTL_debug_keys */
 struct xen_sysctl_debug_keys {
     /* IN variables. */
-    XEN_GUEST_HANDLE_64(char) keys;
+    XEN_GUEST_HANDLE_64(const_char) keys;
     uint32_t nr_keys;
 };
 
@@ -1037,7 +1037,7 @@  struct xen_sysctl_livepatch_op {
  */
 
 struct xen_sysctl_set_parameter {
-    XEN_GUEST_HANDLE_64(char) params;       /* IN: pointer to parameters. */
+    XEN_GUEST_HANDLE_64(const_char) params; /* IN: pointer to parameters. */
     uint16_t size;                          /* IN: size of parameters. */
     uint16_t pad[3];                        /* IN: MUST be zero. */
 };