From: Luca Fancellu <luca.fancellu@arm.com>
To: Elliott Mitchell <ehem+xen@m5p.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>, Wei Liu <wl@xen.org>,
Anthony PERARD <anthony.perard@citrix.com>,
Juergen Gross <jgross@suse.com>
Subject: Re: [PATCH 1/5] tools/libxl: Mark pointer args of many functions constant
Date: Wed, 29 Dec 2021 17:15:25 +0000 [thread overview]
Message-ID: <E320F70F-6BE7-43E9-822D-F175862D0005@arm.com> (raw)
In-Reply-To: <80dd561339dbe54f1ed2c2302ace389e87d445fe.1640590794.git.ehem+xen@m5p.com>
> On 18 Dec 2020, at 21:37, Elliott Mitchell <ehem+xen@m5p.com> 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.
Looks ok to me
Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>
>
> 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 e1acf6648d..b076d7f4a3 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 86556b6113..da2dbd67ad 100644
> --- a/tools/libs/light/libxl_internal.c
> +++ b/tools/libs/light/libxl_internal.c
> @@ -333,7 +333,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;
>
> @@ -345,7 +345,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 37d5c27756..117a98acab 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -2080,9 +2080,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);
>
> @@ -4580,7 +4580,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;
> }
> @@ -4675,22 +4675,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 {
> @@ -4821,12 +4821,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 0630959e76..f40a004e95 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;
> }
> @@ -41,7 +41,7 @@ int 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;
> }
> --
> 2.30.2
>
>
next prev parent reply other threads:[~2021-12-29 17:16 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-27 7:39 [PATCH 0/5] Some misc from my tree Elliott Mitchell
2020-12-10 23:09 ` [PATCH 5/5] tools/xl: Fix potential deallocation bug Elliott Mitchell
2021-12-29 17:20 ` Luca Fancellu
2022-01-05 15:05 ` Anthony PERARD
2022-04-22 22:58 ` Elliott Mitchell
2020-12-18 1:42 ` [PATCH 4/5] tools/xl: Merge down debug/dry-run section of create_domain() Elliott Mitchell
2021-12-29 17:19 ` Luca Fancellu
2020-12-18 1:42 ` [PATCH 3/5] tools/xl: Rename printf_info()/list_domains_details() to dump_by_...() Elliott Mitchell
2021-12-29 17:18 ` Luca Fancellu
2020-12-18 21:32 ` [PATCH 2/5] tools/xl: Mark libxl_domain_config * arg of printf_info_*() const Elliott Mitchell
2021-12-29 17:18 ` Luca Fancellu
2020-12-18 21:37 ` [PATCH 1/5] tools/libxl: Mark pointer args of many functions constant Elliott Mitchell
2021-12-29 17:15 ` Luca Fancellu [this message]
2022-01-05 10:09 ` Anthony PERARD
2022-01-05 13:09 ` Luca Fancellu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E320F70F-6BE7-43E9-822D-F175862D0005@arm.com \
--to=luca.fancellu@arm.com \
--cc=anthony.perard@citrix.com \
--cc=ehem+xen@m5p.com \
--cc=jgross@suse.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.