All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH 2/2] tools/xl: Mark libxl_domain_config * arg of printf_info_*() const
  2021-02-16 17:23 [RESEND PATCH 0/2] Adding const to many libxl/xl functions Elliott Mitchell
@ 2020-12-18 21:32 ` Elliott Mitchell
  2021-04-27 14:54   ` Anthony PERARD
  2020-12-18 21:37 ` [RESEND PATCH 1/2] tools/libxl: Mark pointer args of many functions constant Elliott Mitchell
  1 sibling, 1 reply; 7+ messages in thread
From: Elliott Mitchell @ 2020-12-18 21:32 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Wei Liu, Anthony PERARD

With libxl having gotten a lot more constant, now printf_info_sexp() and
printf_info_one_json() can add consts.  May not be particularly
important, but it is best to mark things constant when they are known to
be so.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
---
 tools/xl/xl.h      | 2 +-
 tools/xl/xl_info.c | 2 +-
 tools/xl/xl_sxp.c  | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/xl/xl.h b/tools/xl/xl.h
index 137a29077c..30a6ddb86f 100644
--- a/tools/xl/xl.h
+++ b/tools/xl/xl.h
@@ -300,7 +300,7 @@ typedef enum {
     DOMAIN_RESTART_SOFT_RESET,   /* Soft reset should be performed */
 } domain_restart_type;
 
-extern void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh);
+extern void printf_info_sexp(int domid, const libxl_domain_config *d_config, FILE *fh);
 extern void apply_global_affinity_masks(libxl_domain_type type,
                                         libxl_bitmap *vcpu_affinity_array,
                                         unsigned int size);
diff --git a/tools/xl/xl_info.c b/tools/xl/xl_info.c
index 8383e4a6df..18c88abf34 100644
--- a/tools/xl/xl_info.c
+++ b/tools/xl/xl_info.c
@@ -59,7 +59,7 @@ static int maybe_printf(const char *fmt, ...)
 }
 
 static yajl_gen_status printf_info_one_json(yajl_gen hand, int domid,
-                                            libxl_domain_config *d_config)
+                                            const libxl_domain_config *d_config)
 {
     yajl_gen_status s;
 
diff --git a/tools/xl/xl_sxp.c b/tools/xl/xl_sxp.c
index 359a001570..d5b9051dfc 100644
--- a/tools/xl/xl_sxp.c
+++ b/tools/xl/xl_sxp.c
@@ -26,13 +26,13 @@
 /* In general you should not add new output to this function since it
  * is intended only for legacy use.
  */
-void printf_info_sexp(int domid, libxl_domain_config *d_config, FILE *fh)
+void printf_info_sexp(int domid, const libxl_domain_config *d_config, FILE *fh)
 {
     int i;
     libxl_dominfo info;
 
-    libxl_domain_create_info *c_info = &d_config->c_info;
-    libxl_domain_build_info *b_info = &d_config->b_info;
+    const libxl_domain_create_info *c_info = &d_config->c_info;
+    const libxl_domain_build_info *b_info = &d_config->b_info;
 
     fprintf(fh, "(domain\n\t(domid %d)\n", domid);
     fprintf(fh, "\t(create_info)\n");
-- 


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445





^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RESEND PATCH 1/2] tools/libxl: Mark pointer args of many functions constant
  2021-02-16 17:23 [RESEND PATCH 0/2] Adding const to many libxl/xl functions Elliott Mitchell
  2020-12-18 21:32 ` [RESEND PATCH 2/2] tools/xl: Mark libxl_domain_config * arg of printf_info_*() const Elliott Mitchell
@ 2020-12-18 21:37 ` Elliott Mitchell
  2021-04-27 14:53   ` Anthony PERARD
  2021-04-28 17:12   ` [SUSPECTED SPAM][RESEND " Anthony PERARD
  1 sibling, 2 replies; 7+ messages in thread
From: Elliott Mitchell @ 2020-12-18 21:37 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Wei Liu, Anthony PERARD

Anything *_is_empty(), *_is_default(), or *_gen_json() is going to be
examining the pointed to thing, not modifying it.  This potentially
results in higher-performance output.  This also allows spreading
constants further, allowing more checking and security.

Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
---
 tools/include/libxl_json.h        | 22 ++++++++++++----------
 tools/libs/light/gentypes.py      |  8 ++++----
 tools/libs/light/libxl_cpuid.c    |  2 +-
 tools/libs/light/libxl_internal.c |  4 ++--
 tools/libs/light/libxl_internal.h | 18 +++++++++---------
 tools/libs/light/libxl_json.c     | 18 ++++++++++--------
 tools/libs/light/libxl_nocpuid.c  |  4 ++--
 7 files changed, 40 insertions(+), 36 deletions(-)

diff --git a/tools/include/libxl_json.h b/tools/include/libxl_json.h
index 260783bfde..63f0e58fe1 100644
--- a/tools/include/libxl_json.h
+++ b/tools/include/libxl_json.h
@@ -23,17 +23,19 @@
 #endif
 
 yajl_gen_status libxl__uint64_gen_json(yajl_gen hand, uint64_t val);
-yajl_gen_status libxl_defbool_gen_json(yajl_gen hand, libxl_defbool *p);
-yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, libxl_uuid *p);
-yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *p);
-yajl_gen_status libxl_bitmap_gen_json(yajl_gen hand, libxl_bitmap *p);
+yajl_gen_status libxl_defbool_gen_json(yajl_gen hand, const libxl_defbool *p);
+yajl_gen_status libxl_uuid_gen_json(yajl_gen hand, const libxl_uuid *p);
+yajl_gen_status libxl_mac_gen_json(yajl_gen hand, const libxl_mac *p);
+yajl_gen_status libxl_bitmap_gen_json(yajl_gen hand, const libxl_bitmap *p);
 yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
-                                                 libxl_cpuid_policy_list *p);
-yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *p);
+                                                 const libxl_cpuid_policy_list *p);
+yajl_gen_status libxl_string_list_gen_json(yajl_gen hand,
+                                           const libxl_string_list *p);
 yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
-                                              libxl_key_value_list *p);
-yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand, libxl_hwcap *p);
-yajl_gen_status libxl_ms_vm_genid_gen_json(yajl_gen hand, libxl_ms_vm_genid *p);
+                                              const libxl_key_value_list *p);
+yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand, const libxl_hwcap *p);
+yajl_gen_status libxl_ms_vm_genid_gen_json(yajl_gen hand,
+                                           const libxl_ms_vm_genid *p);
 
 #include <_libxl_types_json.h>
 
@@ -91,6 +93,6 @@ static inline yajl_gen libxl_yajl_gen_alloc(const yajl_alloc_funcs *allocFuncs)
 #endif /* !HAVE_YAJL_V2 */
 
 yajl_gen_status libxl_domain_config_gen_json(yajl_gen hand,
-                                             libxl_domain_config *p);
+                                             const libxl_domain_config *p);
 
 #endif /* LIBXL_JSON_H */
diff --git a/tools/libs/light/gentypes.py b/tools/libs/light/gentypes.py
index 9a45e45acc..7e02a5366f 100644
--- a/tools/libs/light/gentypes.py
+++ b/tools/libs/light/gentypes.py
@@ -632,7 +632,7 @@ if __name__ == '__main__':
                                                ty.make_arg("p"),
                                                ku.keyvar.type.make_arg(ku.keyvar.name)))
         if ty.json_gen_fn is not None:
-            f.write("%schar *%s_to_json(libxl_ctx *ctx, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p")))
+            f.write("%schar *%s_to_json(libxl_ctx *ctx, const %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p")))
         if ty.json_parse_fn is not None:
             f.write("%sint %s_from_json(libxl_ctx *ctx, %s, const char *s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
         if isinstance(ty, idl.Enumeration):
@@ -662,7 +662,7 @@ if __name__ == '__main__':
 """ % (header_json_define, header_json_define, " ".join(sys.argv)))
 
     for ty in [ty for ty in types if ty.json_gen_fn is not None]:
-        f.write("%syajl_gen_status %s_gen_json(yajl_gen hand, %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
+        f.write("%syajl_gen_status %s_gen_json(yajl_gen hand, const %s);\n" % (ty.hidden(), ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
 
     f.write("\n")
     f.write("""#endif /* %s */\n""" % header_json_define)
@@ -766,13 +766,13 @@ if __name__ == '__main__':
         f.write("\n")
 
     for ty in [t for t in types if t.json_gen_fn is not None]:
-        f.write("yajl_gen_status %s_gen_json(yajl_gen hand, %s)\n" % (ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
+        f.write("yajl_gen_status %s_gen_json(yajl_gen hand, const %s)\n" % (ty.typename, ty.make_arg("p", passby=idl.PASS_BY_REFERENCE)))
         f.write("{\n")
         f.write(libxl_C_type_gen_json(ty, "p"))
         f.write("}\n")
         f.write("\n")
 
-        f.write("char *%s_to_json(libxl_ctx *ctx, %s)\n" % (ty.typename, ty.make_arg("p")))
+        f.write("char *%s_to_json(libxl_ctx *ctx, const %s)\n" % (ty.typename, ty.make_arg("p")))
         f.write("{\n")
         f.write(libxl_C_type_to_json(ty, "p"))
         f.write("}\n")
diff --git a/tools/libs/light/libxl_cpuid.c b/tools/libs/light/libxl_cpuid.c
index 289c59c742..b3e4212edf 100644
--- a/tools/libs/light/libxl_cpuid.c
+++ b/tools/libs/light/libxl_cpuid.c
@@ -14,7 +14,7 @@
 
 #include "libxl_internal.h"
 
-int libxl__cpuid_policy_is_empty(libxl_cpuid_policy_list *pl)
+int libxl__cpuid_policy_is_empty(const libxl_cpuid_policy_list *pl)
 {
     return !libxl_cpuid_policy_list_length(pl);
 }
diff --git a/tools/libs/light/libxl_internal.c b/tools/libs/light/libxl_internal.c
index d93a75533f..32b8788b59 100644
--- a/tools/libs/light/libxl_internal.c
+++ b/tools/libs/light/libxl_internal.c
@@ -332,7 +332,7 @@ _hidden int libxl__parse_mac(const char *s, libxl_mac mac)
     return 0;
 }
 
-_hidden int libxl__compare_macs(libxl_mac *a, libxl_mac *b)
+_hidden int libxl__compare_macs(const libxl_mac *a, const libxl_mac *b)
 {
     int i;
 
@@ -344,7 +344,7 @@ _hidden int libxl__compare_macs(libxl_mac *a, libxl_mac *b)
     return 0;
 }
 
-_hidden int libxl__mac_is_default(libxl_mac *mac)
+_hidden int libxl__mac_is_default(const libxl_mac *mac)
 {
     return (!(*mac)[0] && !(*mac)[1] && !(*mac)[2] &&
             !(*mac)[3] && !(*mac)[4] && !(*mac)[5]);
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index 028bc013d9..e3df881d08 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -2073,9 +2073,9 @@ struct libxl__xen_console_reader {
 /* parse the string @s as a sequence of 6 colon separated bytes in to @mac */
 _hidden int libxl__parse_mac(const char *s, libxl_mac mac);
 /* compare mac address @a and @b. 0 if the same, -ve if a<b and +ve if a>b */
-_hidden int libxl__compare_macs(libxl_mac *a, libxl_mac *b);
+_hidden int libxl__compare_macs(const libxl_mac *a, const libxl_mac *b);
 /* return true if mac address is all zero (the default value) */
-_hidden int libxl__mac_is_default(libxl_mac *mac);
+_hidden int libxl__mac_is_default(const libxl_mac *mac);
 /* init a recursive mutex */
 _hidden int libxl__init_recursive_mutex(libxl_ctx *ctx, pthread_mutex_t *lock);
 
@@ -4571,7 +4571,7 @@ _hidden int libxl__ms_vm_genid_set(libxl__gc *gc, uint32_t domid,
 #define LIBXL__DEFBOOL_STR_DEFAULT "<default>"
 #define LIBXL__DEFBOOL_STR_FALSE   "False"
 #define LIBXL__DEFBOOL_STR_TRUE    "True"
-static inline int libxl__defbool_is_default(libxl_defbool *db)
+static inline int libxl__defbool_is_default(const libxl_defbool *db)
 {
     return !db->val;
 }
@@ -4666,22 +4666,22 @@ int libxl__random_bytes(libxl__gc *gc, uint8_t *buf, size_t len);
 #include "_libxl_types_internal_private.h"
 
 /* This always return false, there's no "default value" for hw cap */
-static inline int libxl__hwcap_is_default(libxl_hwcap *hwcap)
+static inline int libxl__hwcap_is_default(const libxl_hwcap *hwcap)
 {
     return 0;
 }
 
-static inline int libxl__string_list_is_empty(libxl_string_list *psl)
+static inline int libxl__string_list_is_empty(const libxl_string_list *psl)
 {
     return !libxl_string_list_length(psl);
 }
 
-static inline int libxl__key_value_list_is_empty(libxl_key_value_list *pkvl)
+static inline int libxl__key_value_list_is_empty(const libxl_key_value_list *pkvl)
 {
     return !libxl_key_value_list_length(pkvl);
 }
 
-int libxl__cpuid_policy_is_empty(libxl_cpuid_policy_list *pl);
+int libxl__cpuid_policy_is_empty(const libxl_cpuid_policy_list *pl);
 
 /* Portability note: a proper flock(2) implementation is required */
 typedef struct {
@@ -4812,12 +4812,12 @@ void* libxl__device_list(libxl__gc *gc, const libxl__device_type *dt,
 void libxl__device_list_free(const libxl__device_type *dt,
                              void *list, int num);
 
-static inline bool libxl__timer_mode_is_default(libxl_timer_mode *tm)
+static inline bool libxl__timer_mode_is_default(const libxl_timer_mode *tm)
 {
     return *tm == LIBXL_TIMER_MODE_DEFAULT;
 }
 
-static inline bool libxl__string_is_default(char **s)
+static inline bool libxl__string_is_default(char *const *s)
 {
     return *s == NULL;
 }
diff --git a/tools/libs/light/libxl_json.c b/tools/libs/light/libxl_json.c
index 9b8ef2cab9..88e81f9905 100644
--- a/tools/libs/light/libxl_json.c
+++ b/tools/libs/light/libxl_json.c
@@ -95,7 +95,7 @@ yajl_gen_status libxl__yajl_gen_enum(yajl_gen hand, const char *str)
  * YAJL generators for builtin libxl types.
  */
 yajl_gen_status libxl_defbool_gen_json(yajl_gen hand,
-                                       libxl_defbool *db)
+                                       const libxl_defbool *db)
 {
     return libxl__yajl_gen_asciiz(hand, libxl_defbool_to_string(*db));
 }
@@ -137,7 +137,7 @@ int libxl__bool_parse_json(libxl__gc *gc, const libxl__json_object *o,
 }
 
 yajl_gen_status libxl_uuid_gen_json(yajl_gen hand,
-                                    libxl_uuid *uuid)
+                                    const libxl_uuid *uuid)
 {
     char buf[LIBXL_UUID_FMTLEN+1];
     snprintf(buf, sizeof(buf), LIBXL_UUID_FMT, LIBXL_UUID_BYTES((*uuid)));
@@ -154,7 +154,7 @@ int libxl__uuid_parse_json(libxl__gc *gc, const libxl__json_object *o,
 }
 
 yajl_gen_status libxl_bitmap_gen_json(yajl_gen hand,
-                                      libxl_bitmap *bitmap)
+                                      const libxl_bitmap *bitmap)
 {
     yajl_gen_status s;
     int i;
@@ -208,7 +208,7 @@ int libxl__bitmap_parse_json(libxl__gc *gc, const libxl__json_object *o,
 }
 
 yajl_gen_status libxl_key_value_list_gen_json(yajl_gen hand,
-                                              libxl_key_value_list *pkvl)
+                                              const libxl_key_value_list *pkvl)
 {
     libxl_key_value_list kvl = *pkvl;
     yajl_gen_status s;
@@ -269,7 +269,8 @@ int libxl__key_value_list_parse_json(libxl__gc *gc, const libxl__json_object *o,
     return 0;
 }
 
-yajl_gen_status libxl_string_list_gen_json(yajl_gen hand, libxl_string_list *pl)
+yajl_gen_status libxl_string_list_gen_json(yajl_gen hand,
+                                           const libxl_string_list *pl)
 {
     libxl_string_list l = *pl;
     yajl_gen_status s;
@@ -322,7 +323,7 @@ int libxl__string_list_parse_json(libxl__gc *gc, const libxl__json_object *o,
     return 0;
 }
 
-yajl_gen_status libxl_mac_gen_json(yajl_gen hand, libxl_mac *mac)
+yajl_gen_status libxl_mac_gen_json(yajl_gen hand, const libxl_mac *mac)
 {
     char buf[LIBXL_MAC_FMTLEN+1];
     snprintf(buf, sizeof(buf), LIBXL_MAC_FMT, LIBXL_MAC_BYTES((*mac)));
@@ -339,7 +340,7 @@ int libxl__mac_parse_json(libxl__gc *gc, const libxl__json_object *o,
 }
 
 yajl_gen_status libxl_hwcap_gen_json(yajl_gen hand,
-                                     libxl_hwcap *p)
+                                     const libxl_hwcap *p)
 {
     yajl_gen_status s;
     int i;
@@ -377,7 +378,8 @@ int libxl__hwcap_parse_json(libxl__gc *gc, const libxl__json_object *o,
     return 0;
 }
 
-yajl_gen_status libxl_ms_vm_genid_gen_json(yajl_gen hand, libxl_ms_vm_genid *p)
+yajl_gen_status libxl_ms_vm_genid_gen_json(yajl_gen hand,
+                                           const libxl_ms_vm_genid *p)
 {
     yajl_gen_status s;
     int i;
diff --git a/tools/libs/light/libxl_nocpuid.c b/tools/libs/light/libxl_nocpuid.c
index f47336565b..73580351b3 100644
--- a/tools/libs/light/libxl_nocpuid.c
+++ b/tools/libs/light/libxl_nocpuid.c
@@ -14,7 +14,7 @@
 
 #include "libxl_internal.h"
 
-int libxl__cpuid_policy_is_empty(libxl_cpuid_policy_list *pl)
+int libxl__cpuid_policy_is_empty(const libxl_cpuid_policy_list *pl)
 {
     return 1;
 }
@@ -40,7 +40,7 @@ void libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid, bool restore,
 }
 
 yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
-                                libxl_cpuid_policy_list *pcpuid)
+                                const libxl_cpuid_policy_list *pcpuid)
 {
     return 0;
 }
-- 


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445





^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [RESEND PATCH 0/2] Adding const to many libxl/xl functions
@ 2021-02-16 17:23 Elliott Mitchell
  2020-12-18 21:32 ` [RESEND PATCH 2/2] tools/xl: Mark libxl_domain_config * arg of printf_info_*() const Elliott Mitchell
  2020-12-18 21:37 ` [RESEND PATCH 1/2] tools/libxl: Mark pointer args of many functions constant Elliott Mitchell
  0 siblings, 2 replies; 7+ messages in thread
From: Elliott Mitchell @ 2021-02-16 17:23 UTC (permalink / raw)
  To: xen-devel; +Cc: Ian Jackson, Wei Liu, Anthony PERARD

The rest of the series seems hopeless for stable, so right now I'm merely
resending the 2 which are simpler.  During the full series I came across
a bunch of xl and then libxl functions which could have arguments
declared const.

These are the input arguments of *_is_empty() and *_is_default(), which
are merely read from.  There are also *_gen_json() functions where the
yajl handle needs to be writeable, but the input data structure isn't
modified.

The second is merely spreading these further outwards.  Once libxl marks
its function's arguments const, portions of `xl` can similarly have
functions marked const.

NOTE: Order is important on these two!

Elliott Mitchell (2):
  tools/libxl: Mark pointer args of many functions constant
  tools/xl: Mark libxl_domain_config * arg of printf_info_*() const

 tools/include/libxl_json.h        | 22 ++++++++++++----------
 tools/libs/light/gentypes.py      |  8 ++++----
 tools/libs/light/libxl_cpuid.c    |  2 +-
 tools/libs/light/libxl_internal.c |  4 ++--
 tools/libs/light/libxl_internal.h | 18 +++++++++---------
 tools/libs/light/libxl_json.c     | 18 ++++++++++--------
 tools/libs/light/libxl_nocpuid.c  |  4 ++--
 tools/xl/xl.h                     |  2 +-
 tools/xl/xl_info.c                |  2 +-
 tools/xl/xl_sxp.c                 |  6 +++---
 10 files changed, 45 insertions(+), 41 deletions(-)

-- 


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RESEND PATCH 1/2] tools/libxl: Mark pointer args of many functions constant
  2020-12-18 21:37 ` [RESEND PATCH 1/2] tools/libxl: Mark pointer args of many functions constant Elliott Mitchell
@ 2021-04-27 14:53   ` Anthony PERARD
  2021-04-28 17:12   ` [SUSPECTED SPAM][RESEND " Anthony PERARD
  1 sibling, 0 replies; 7+ messages in thread
From: Anthony PERARD @ 2021-04-27 14:53 UTC (permalink / raw)
  To: Elliott Mitchell; +Cc: xen-devel, Ian Jackson, Wei Liu

On Fri, Dec 18, 2020 at 01:37:44PM -0800, Elliott Mitchell wrote:
> Anything *_is_empty(), *_is_default(), or *_gen_json() is going to be
> examining the pointed to thing, not modifying it.  This potentially
> results in higher-performance output.  This also allows spreading
> constants further, allowing more checking and security.
> 
> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
> ---
> diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
> index 028bc013d9..e3df881d08 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
[..]
> -static inline bool libxl__string_is_default(char **s)
> +static inline bool libxl__string_is_default(char *const *s)

That looks weird to not also have "char" been "const", but I've tried to
change that and the compiler wasn't cooperative so I guess that's fine.


> diff --git a/tools/libs/light/libxl_nocpuid.c b/tools/libs/light/libxl_nocpuid.c
> index f47336565b..73580351b3 100644
> --- a/tools/libs/light/libxl_nocpuid.c
> +++ b/tools/libs/light/libxl_nocpuid.c
> @@ -40,7 +40,7 @@ void libxl__cpuid_legacy(libxl_ctx *ctx, uint32_t domid, bool restore,
>  }
>  
>  yajl_gen_status libxl_cpuid_policy_list_gen_json(yajl_gen hand,
> -                                libxl_cpuid_policy_list *pcpuid)
> +                                const libxl_cpuid_policy_list *pcpuid)

That change is missing in libxl_cpuid.c.


The rest looks fine, so once libxl_cpuid.c is fix, you can have my Reviewed-by.

Thanks,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RESEND PATCH 2/2] tools/xl: Mark libxl_domain_config * arg of printf_info_*() const
  2020-12-18 21:32 ` [RESEND PATCH 2/2] tools/xl: Mark libxl_domain_config * arg of printf_info_*() const Elliott Mitchell
@ 2021-04-27 14:54   ` Anthony PERARD
  0 siblings, 0 replies; 7+ messages in thread
From: Anthony PERARD @ 2021-04-27 14:54 UTC (permalink / raw)
  To: Elliott Mitchell; +Cc: xen-devel, Ian Jackson, Wei Liu

On Fri, Dec 18, 2020 at 01:32:33PM -0800, Elliott Mitchell wrote:
> With libxl having gotten a lot more constant, now printf_info_sexp() and
> printf_info_one_json() can add consts.  May not be particularly
> important, but it is best to mark things constant when they are known to
> be so.
> 
> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [SUSPECTED SPAM][RESEND PATCH 1/2] tools/libxl: Mark pointer args of many functions constant
  2020-12-18 21:37 ` [RESEND PATCH 1/2] tools/libxl: Mark pointer args of many functions constant Elliott Mitchell
  2021-04-27 14:53   ` Anthony PERARD
@ 2021-04-28 17:12   ` Anthony PERARD
  2021-04-28 17:29     ` Andrew Cooper
  1 sibling, 1 reply; 7+ messages in thread
From: Anthony PERARD @ 2021-04-28 17:12 UTC (permalink / raw)
  To: Elliott Mitchell; +Cc: xen-devel, Ian Jackson, Wei Liu

On Fri, Dec 18, 2020 at 01:37:44PM -0800, Elliott Mitchell wrote:
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -2073,9 +2073,9 @@ struct libxl__xen_console_reader {
>  /* parse the string @s as a sequence of 6 colon separated bytes in to @mac */
>  _hidden int libxl__parse_mac(const char *s, libxl_mac mac);
>  /* compare mac address @a and @b. 0 if the same, -ve if a<b and +ve if a>b */
> -_hidden int libxl__compare_macs(libxl_mac *a, libxl_mac *b);
> +_hidden int libxl__compare_macs(const libxl_mac *a, const libxl_mac *b);
>  /* return true if mac address is all zero (the default value) */
> -_hidden int libxl__mac_is_default(libxl_mac *mac);
> +_hidden int libxl__mac_is_default(const libxl_mac *mac);

It turns out that older version of gcc complains about the const of
libxl_mac:


    libxl_nic.c: In function 'libxl_mac_to_device_nic':
    libxl_nic.c:40:9: error: passing argument 1 of 'libxl__compare_macs' from incompatible pointer type [-Werror]
             if (!libxl__compare_macs(&mac_n, &nics[i].mac)) {
             ^
    In file included from libxl_nic.c:18:0:
    libxl_internal.h:2076:13: note: expected 'const uint8_t (*)[6]' but argument is of type 'uint8_t (*)[6]'
     _hidden int libxl__compare_macs(const libxl_mac *a, const libxl_mac *b);
                 ^
    libxl_nic.c:40:9: error: passing argument 2 of 'libxl__compare_macs' from incompatible pointer type [-Werror]
             if (!libxl__compare_macs(&mac_n, &nics[i].mac)) {
             ^
    In file included from libxl_nic.c:18:0:
    libxl_internal.h:2076:13: note: expected 'const uint8_t (*)[6]' but argument is of type 'uint8_t (*)[6]'
     _hidden int libxl__compare_macs(const libxl_mac *a, const libxl_mac *b);
                 ^
    libxl_nic.c: In function 'libxl__device_nic_setdefault':
    libxl_nic.c:69:5: error: passing argument 1 of 'libxl__mac_is_default' from incompatible pointer type [-Werror]
         if (libxl__mac_is_default(&nic->mac)) {
         ^
    In file included from libxl_nic.c:18:0:
    libxl_internal.h:2078:13: note: expected 'const uint8_t (*)[6]' but argument is of type 'uint8_t (*)[6]'
     _hidden int libxl__mac_is_default(const libxl_mac *mac);
                 ^
    cc1: all warnings being treated as errors


That happens on ubuntu trusty, debian jessie, and centos 7.

Cheers,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [SUSPECTED SPAM][RESEND PATCH 1/2] tools/libxl: Mark pointer args of many functions constant
  2021-04-28 17:12   ` [SUSPECTED SPAM][RESEND " Anthony PERARD
@ 2021-04-28 17:29     ` Andrew Cooper
  0 siblings, 0 replies; 7+ messages in thread
From: Andrew Cooper @ 2021-04-28 17:29 UTC (permalink / raw)
  To: Anthony PERARD, Elliott Mitchell; +Cc: xen-devel, Ian Jackson, Wei Liu

On 28/04/2021 18:12, Anthony PERARD wrote:
> On Fri, Dec 18, 2020 at 01:37:44PM -0800, Elliott Mitchell wrote:
>> --- a/tools/libs/light/libxl_internal.h
>> +++ b/tools/libs/light/libxl_internal.h
>> @@ -2073,9 +2073,9 @@ struct libxl__xen_console_reader {
>>  /* parse the string @s as a sequence of 6 colon separated bytes in to @mac */
>>  _hidden int libxl__parse_mac(const char *s, libxl_mac mac);
>>  /* compare mac address @a and @b. 0 if the same, -ve if a<b and +ve if a>b */
>> -_hidden int libxl__compare_macs(libxl_mac *a, libxl_mac *b);
>> +_hidden int libxl__compare_macs(const libxl_mac *a, const libxl_mac *b);
>>  /* return true if mac address is all zero (the default value) */
>> -_hidden int libxl__mac_is_default(libxl_mac *mac);
>> +_hidden int libxl__mac_is_default(const libxl_mac *mac);
> It turns out that older version of gcc complains about the const of
> libxl_mac:
>
>
>     libxl_nic.c: In function 'libxl_mac_to_device_nic':
>     libxl_nic.c:40:9: error: passing argument 1 of 'libxl__compare_macs' from incompatible pointer type [-Werror]
>              if (!libxl__compare_macs(&mac_n, &nics[i].mac)) {
>              ^
>     In file included from libxl_nic.c:18:0:
>     libxl_internal.h:2076:13: note: expected 'const uint8_t (*)[6]' but argument is of type 'uint8_t (*)[6]'
>      _hidden int libxl__compare_macs(const libxl_mac *a, const libxl_mac *b);
>                  ^
>     libxl_nic.c:40:9: error: passing argument 2 of 'libxl__compare_macs' from incompatible pointer type [-Werror]
>              if (!libxl__compare_macs(&mac_n, &nics[i].mac)) {
>              ^
>     In file included from libxl_nic.c:18:0:
>     libxl_internal.h:2076:13: note: expected 'const uint8_t (*)[6]' but argument is of type 'uint8_t (*)[6]'
>      _hidden int libxl__compare_macs(const libxl_mac *a, const libxl_mac *b);
>                  ^
>     libxl_nic.c: In function 'libxl__device_nic_setdefault':
>     libxl_nic.c:69:5: error: passing argument 1 of 'libxl__mac_is_default' from incompatible pointer type [-Werror]
>          if (libxl__mac_is_default(&nic->mac)) {
>          ^
>     In file included from libxl_nic.c:18:0:
>     libxl_internal.h:2078:13: note: expected 'const uint8_t (*)[6]' but argument is of type 'uint8_t (*)[6]'
>      _hidden int libxl__mac_is_default(const libxl_mac *mac);
>                  ^
>     cc1: all warnings being treated as errors

That's because the typedef for libxl_mac violates one of the well known
C beginner mistakes, by being a plain array behind the scenes.

A prototype of 'const libxl_mac mac[]' might fair better, except it
makes the code even more confusing to follow.

~Andrew



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-04-28 17:30 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16 17:23 [RESEND PATCH 0/2] Adding const to many libxl/xl functions Elliott Mitchell
2020-12-18 21:32 ` [RESEND PATCH 2/2] tools/xl: Mark libxl_domain_config * arg of printf_info_*() const Elliott Mitchell
2021-04-27 14:54   ` Anthony PERARD
2020-12-18 21:37 ` [RESEND PATCH 1/2] tools/libxl: Mark pointer args of many functions constant Elliott Mitchell
2021-04-27 14:53   ` Anthony PERARD
2021-04-28 17:12   ` [SUSPECTED SPAM][RESEND " Anthony PERARD
2021-04-28 17:29     ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.