Skip to content

Commit 7fcaaef

Browse files
authored
zerocheck: fix +2 GiB issue, add tests (#6721)
The issue was that the 61-bit ocaml value was being truncated to 32 bits.
2 parents 0286ff0 + 477518e commit 7fcaaef

File tree

8 files changed

+66
-14
lines changed

8 files changed

+66
-14
lines changed

ocaml/idl/datamodel_lifecycle.ml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,9 @@ let prototyped_of_message = function
226226
| "VTPM", "create" ->
227227
Some "22.26.0"
228228
| "host", "update_firewalld_service_status" ->
229-
Some "25.30.0-next"
229+
Some "25.33.0-next"
230+
| "host", "get_tracked_user_agents" ->
231+
Some "25.33.0-next"
230232
| "host", "set_ssh_auto_mode" ->
231233
Some "25.27.0"
232234
| "host", "set_console_idle_timeout" ->
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
(library
22
(public_name xapi-stdext-zerocheck)
33
(name xapi_stdext_zerocheck)
4+
(modules :standard \ zerocheck_test)
45
(foreign_stubs (language c) (names zerocheck_stub))
56
)
7+
8+
(test
9+
(name zerocheck_test)
10+
(package xapi-stdext-zerocheck)
11+
(modules zerocheck_test)
12+
(libraries alcotest xapi-stdext-zerocheck)
13+
)

ocaml/libs/xapi-stdext/lib/xapi-stdext-zerocheck/zerocheck.ml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,7 @@
1111
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
1212
* GNU Lesser General Public License for more details.
1313
*)
14-
external is_all_zeros : string -> int -> bool = "is_all_zeros"
14+
15+
external is_all_zeros_in_length : string -> int -> bool = "is_all_zeros"
16+
17+
let is_all_zeros str = is_all_zeros_in_length str (String.length str)

ocaml/libs/xapi-stdext/lib/xapi-stdext-zerocheck/zerocheck.mli

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@
1212
* GNU Lesser General Public License for more details.
1313
*)
1414

15-
external is_all_zeros : string -> int -> bool = "is_all_zeros"
16-
(** [is_all_zeroes x len] returns true if the substring is all zeroes *)
15+
val is_all_zeros : string -> bool
16+
(** [is_all_zeroes x] returns whether [x] contains only zeroes *)

ocaml/libs/xapi-stdext/lib/xapi-stdext-zerocheck/zerocheck_stub.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ value is_all_zeros(value string, value length)
2323
{
2424
CAMLparam2(string, length);
2525
const char *s = String_val(string);
26-
unsigned int *p;
27-
int len = Int_val(length);
28-
int i;
26+
unsigned const int *p;
27+
long len = Long_val(length);
28+
long i;
2929

3030
p = (unsigned int *) s;
3131
for (i = len / 4; i > 0; i--)
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
(* Copyright (C) 2025 Vates
2+
3+
This program is free software; you can redistribute it and/or modify
4+
it under the terms of the GNU Lesser General Public License as published
5+
by the Free Software Foundation; version 2.1 only. with the special
6+
exception on linking described in file LICENSE.
7+
8+
This program is distributed in the hope that it will be useful,
9+
but WITHOUT ANY WARRANTY; without even the implied warranty of
10+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11+
GNU Lesser General Public License for more details.
12+
*)
13+
14+
module Zerocheck = Xapi_stdext_zerocheck.Zerocheck
15+
16+
module Str = struct
17+
let big_non_zero =
18+
let spec = [("2 GiBs", 2_147_483_647); ("2 GiBs + 1", 2147483647 + 1)] in
19+
let test size () =
20+
let non_zeroes = Bytes.make size '\x00' in
21+
Bytes.set non_zeroes (size - 1) '\x01' ;
22+
let non_zeroes = Bytes.unsafe_to_string non_zeroes in
23+
let expected = true in
24+
let actual = not (Zerocheck.is_all_zeros non_zeroes) in
25+
Alcotest.(check bool) "The last byte is not zero" expected actual
26+
in
27+
List.map (fun (name, size) -> (name, `Quick, test size)) spec
28+
29+
let big_zero =
30+
let test () =
31+
let size = 2147483647 + 1 in
32+
let zeroes = String.make size '\x00' in
33+
let expected = true in
34+
let actual = Zerocheck.is_all_zeros zeroes in
35+
Alcotest.(check bool) "All bytes are zero" expected actual
36+
in
37+
[("2 GiBs + 1", `Quick, test)]
38+
39+
let tests =
40+
[("String: Not all zeroes", big_non_zero); ("String: all zeroes", big_zero)]
41+
end
42+
43+
let () = Alcotest.run "Zerocheck" Str.tests

ocaml/libs/xapi-stdext/lib/xapi-stdext-zerocheck/zerocheck_test.mli

Whitespace-only changes.

ocaml/xapi/stream_vdi.ml

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -295,13 +295,9 @@ let send_all refresh_session ofd ~__context rpc session_id
295295
in
296296
Unixext.really_read ifd buffer 0 this_chunk ;
297297
if
298-
(not
299-
(Zerocheck.is_all_zeros
300-
(Bytes.unsafe_to_string buffer)
301-
this_chunk
302-
)
303-
)
304-
|| first_or_last
298+
first_or_last
299+
|| not
300+
(Zerocheck.is_all_zeros (Bytes.unsafe_to_string buffer))
305301
then (
306302
last_transmission_time := now ;
307303
write_block ~__context filename buffer ofd this_chunk

0 commit comments

Comments
 (0)