[m-rev.] for review: access bytes in bitmaps as uint8s

Zoltan Somogyi zoltan.somogyi at runbox.com
Wed Jan 3 05:34:38 AEDT 2018



On Sat, 30 Dec 2017 00:56:42 -0500 (EST), Julien Fischer <jfischer at opturion.com> wrote:
> @@ -393,10 +413,16 @@
>   :- mode to_string(in) = out is det.
> 
>       % Convert a string created by to_string back into a bitmap.
> +    % Fails if the string is not of the form created by to_string.
>       %
>   :- func from_string(string) = bitmap.
>   :- mode from_string(in) = bitmap_uo is semidet.

Unrelated to this change, but it would be nicer to have semidet code
as predicates, not functions.

> +get_uint8(BM, N) = U8 :-
> +    ( if N >= 0, in_range(BM, N * bits_per_byte + bits_per_byte - 1) then
> +        U8 = unsafe_get_uint8(BM, N)
> +    else
> +        throw_byte_bounds_error(BM, "bitmap.get_uint8", N)
> +    ).

Shouldn't this call byte_in_range, instead of inlining it by hand?
That would also avoid the double test of N >= 0.

Same issue with set_uint8.

> +:- pragma foreign_proc("Erlang",
> +    unsafe_get_uint8(BM::in, N::in) = (U8::out),
> +    [will_not_call_mercury, promise_pure, thread_safe],
> +"
> +    {Bin, _} = BM,
> +    <<_:N/binary, U8/integer, _/binary>> = Bin
> +").

I don't understand this, or its set version.

Otherwise the diff is fine.

Zoltan.




More information about the reviews mailing list