[m-rev.] for review: add conversion fucntions from bytes to 16-bit integers

Peter Wang novalazy at gmail.com
Tue Oct 10 14:21:45 AEDT 2017


Hi Julien,

On Mon, 9 Oct 2017 21:06:54 -0400 (EDT), Julien Fischer <jfischer at opturion.com> wrote:
> diff --git a/library/int16.m b/library/int16.m
> index 4399830..4ce54cf 100644
> --- a/library/int16.m
> +++ b/library/int16.m
> @@ -37,6 +37,18 @@
> 
>   :- func to_int(int16) = int.
> 
> +    % from_bytes_le(LSB, MSB) = I16:
> +    % I16 is the int16 whose least and most significant bytes are given by the
> +    % uint8s LSB and MSB respectively.
> +    %
> +:- func from_bytes_le(uint8, uint8) = int16.
> +
> +    % from_bytes_be(MSB, LSB) = I16:
> +    % I16 is the int16 whose least and most significant bytes are given by the
> +    % uint8s LSB and MSB respectively.
> +    %
> +:- func from_bytes_be(uint8, uint8) = int16.

Are the separate le/be functions clearer than reversing the arguments at
the call site? I suggest keeping just the BE version as from_bytes.
Perhaps you expect to use from_bytes_le as a higher-order term?

We also need to specify what happens on overflow.

> @@ -316,6 +328,44 @@ cast_from_uint16(_) = _ :-
> 
>   %---------------------------------------------------------------------------%
> 
> +:- pragma foreign_proc("C",
> +    from_bytes_le(LSB::in, MSB::in) = (I16::out),
> +    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail],
> +"
> +    unsigned char *int16_bytes = (unsigned char *) &I16;
> +#if defined(MR_BIG_ENDIAN)
> +    int16_bytes[0] = MSB;
> +    int16_bytes[1] = LSB;
> +#else
> +    int16_bytes[0] = LSB;
> +    int16_bytes[1] = MSB;
> +#endif
> +").

Is this to avoid undefined behaviour if shifting overflows?
Please add a comment about it.

Peter


More information about the reviews mailing list