Re: [PATCH 2/5] libmultipath: change reservation_key to a uint64_t

[Martin Wilck <mwilck@suse.com>]

On Fri, 2017-09-08 at 13:45 -0500, Benjamin Marzinski wrote:
> The reservation key is currently being stored as any array of 8
> unsigned
> chars.  This is exactly the same in-memory representation as a big
> endian 64 bit integer. However, the code for dealing with a big
> endian
> 64 bit integer is much simpler, so switch to use that instead.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmpathpersist/mpath_persist.c | 23 +++++++----------------
>  libmultipath/byteorder.h        | 36
> ++++++++++++++++++++++++++++++++++++
>  libmultipath/config.c           |  3 ---
>  libmultipath/config.h           |  6 ++++--
>  libmultipath/dict.c             | 31 ++++---------------------------
>  libmultipath/propsel.c          |  6 +++---
>  libmultipath/structs.h          |  5 ++++-
>  multipathd/main.c               | 23 +++++------------------
>  8 files changed, 63 insertions(+), 70 deletions(-)
>  create mode 100644 libmultipath/byteorder.h
> 
>  
>  	char * prio_name;
>  	char * prio_args;
> -	unsigned char * reservation_key;
> +	uint64_t  reservation_key; /* stored in big-endian format */

I would prefer if you could encapsulate this into a type that can't be
implicitly converted to an int, such as

struct res_key {
	uint64_t _v;
} reservation_key;

together with macros like

#define reskey_to_uint64(r) be64_to_cpu((r)->_v)
#define set_reskey_from_uint64(r, x) do { (r)->_v = (x); } while (0)

Using be64_to_cpu is all too easily forgotten, which leads to nasty
bugs.

Apart from that, ack.

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel