Message ID | 20180622231516.18806-4-mwilck@suse.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Delegated to: | christophe varoqui |
Headers | show |
On 06/22/18 16:16, Martin Wilck wrote: > - sa_key = 0; > - for (i = 0; i < 8; ++i){ > - if (i > 0) > - sa_key <<= 8; > - sa_key |= paramp->sa_key[i]; > - } > + sa_key = be64_to_cpu(*(uint64_t *)¶mp->sa_key[0]); Have you considered to use get_unaligned_be64() instead of the above construct with casts? Thanks, Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Fri, 2018-06-22 at 16:19 -0700, Bart Van Assche wrote: > On 06/22/18 16:16, Martin Wilck wrote: > > - sa_key = 0; > > - for (i = 0; i < 8; ++i){ > > - if (i > 0) > > - sa_key <<= 8; > > - sa_key |= paramp->sa_key[i]; > > - } > > + sa_key = be64_to_cpu(*(uint64_t *)¶mp- > > >sa_key[0]); > > Have you considered to use get_unaligned_be64() instead of the above > construct with casts? Yes, I did. I opted for this construct for patch brevity (and admittedly, lazyness), because get_unaligned_be64() doesn't exist yet. I can change that of course if you insist. Thanks, Martin
On 06/22/18 16:27, Martin Wilck wrote: > On Fri, 2018-06-22 at 16:19 -0700, Bart Van Assche wrote: >> On 06/22/18 16:16, Martin Wilck wrote: >>> - sa_key = 0; >>> - for (i = 0; i < 8; ++i){ >>> - if (i > 0) >>> - sa_key <<= 8; >>> - sa_key |= paramp->sa_key[i]; >>> - } >>> + sa_key = be64_to_cpu(*(uint64_t *)¶mp- >>>> sa_key[0]); >> >> Have you considered to use get_unaligned_be64() instead of the above >> construct with casts? > > Yes, I did. I opted for this construct for patch brevity (and > admittedly, lazyness), because get_unaligned_be64() doesn't exist yet. > I can change that of course if you insist. Adding an implementation of get_unaligned_be64() and introducing it in the above code would be highly appreciated. I think that will improve readability. Thanks, Bart. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Fri, 2018-06-22 at 16:48 -0700, Bart Van Assche wrote: > > Adding an implementation of get_unaligned_be64() and introducing it > in > the above code would be highly appreciated. I think that will > improve > readability. ok, will do. Martin
diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c index 907a17c9..12dad7a7 100644 --- a/libmpathpersist/mpath_persist.c +++ b/libmpathpersist/mpath_persist.c @@ -543,12 +543,7 @@ int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, } if (!rollback && (thread[i].param.status == MPATH_PR_RESERV_CONFLICT)){ rollback = 1; - sa_key = 0; - for (i = 0; i < 8; ++i){ - if (i > 0) - sa_key <<= 8; - sa_key |= paramp->sa_key[i]; - } + sa_key = be64_to_cpu(*(uint64_t *)¶mp->sa_key[0]); status = MPATH_PR_RESERV_CONFLICT ; } if (!rollback && (status == MPATH_PR_SUCCESS)){ diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c index 347f21b2..76271ff1 100644 --- a/libmpathpersist/mpath_pr_ioctl.c +++ b/libmpathpersist/mpath_pr_ioctl.c @@ -485,24 +485,12 @@ int mpath_isLittleEndian(void) void mpath_reverse_uint16_byteorder(uint16_t *num) { - uint16_t byte0, byte1; - - byte0 = (*num & 0x000000FF) >> 0 ; - byte1 = (*num & 0x0000FF00) >> 8 ; - - *num = ((byte0 << 8) | (byte1 << 0)); + *num = get_unaligned_be16(num); } void mpath_reverse_uint32_byteorder(uint32_t *num) { - uint32_t byte0, byte1, byte2, byte3; - - byte0 = (*num & 0x000000FF) >> 0 ; - byte1 = (*num & 0x0000FF00) >> 8 ; - byte2 = (*num & 0x00FF0000) >> 16 ; - byte3 = (*num & 0xFF000000) >> 24 ; - - *num = ((byte0 << 24) | (byte1 << 16) | (byte2 << 8) | (byte3 << 0)); + *num = get_unaligned_be32(num); } void
This code was obviously intended to convert big-endian data from PRIN to CPU endianness. It doesn't work on big endian systems. Signed-off-by: Martin Wilck <mwilck@suse.com> --- libmpathpersist/mpath_persist.c | 7 +------ libmpathpersist/mpath_pr_ioctl.c | 16 ++-------------- 2 files changed, 3 insertions(+), 20 deletions(-)