Skip to content

Commit

Permalink
Check index before using it removing an element from Imperative prior…
Browse files Browse the repository at this point in the history
…ity queue

The queue uses an Array larger than the current size.
It's possible that code check the index much later returning error but also
setting the queue in an invalid state.
So to avoid structure corruption check the index as soon as possible.

Signed-off-by: Frediano Ziglio <[email protected]>
  • Loading branch information
freddy77 committed Dec 3, 2024
1 parent 9e3ad1c commit cd98e9d
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 5 deletions.
10 changes: 5 additions & 5 deletions ocaml/libs/xapi-stdext/lib/xapi-stdext-threads/dune
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
(library
(public_name xapi-stdext-threads)
(name xapi_stdext_threads)
(modules :standard \ ipq scheduler threadext_test)
(modules :standard \ ipq scheduler threadext_test ipq_test)
(libraries
threads.posix
unix
Expand All @@ -16,9 +16,9 @@
(libraries mtime mtime.clock threads.posix unix xapi-log xapi-stdext-threads)
)

(test
(name threadext_test)
(tests
(names threadext_test ipq_test)
(package xapi-stdext-threads)
(modules threadext_test)
(libraries xapi_stdext_threads alcotest mtime.clock.os mtime fmt threads.posix)
(modules threadext_test ipq_test)
(libraries xapi_stdext_threads alcotest mtime.clock.os mtime fmt threads.posix xapi_stdext_threads_scheduler)
)
2 changes: 2 additions & 0 deletions ocaml/libs/xapi-stdext/lib/xapi-stdext-threads/ipq.ml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ let maximum h =

let remove h s =
if h.size <= 0 then raise EmptyHeap ;
if s < 0 || s >= h.size then
invalid_arg (Printf.sprintf "%s: index %d out of bounds" __FUNCTION__ s) ;
let n = h.size - 1 in
h.size <- n ;
let d = h.data in
Expand Down
40 changes: 40 additions & 0 deletions ocaml/libs/xapi-stdext/lib/xapi-stdext-threads/ipq_test.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
(*
* Copyright (C) 2024 Cloud Software Group
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU Lesser General Public License as published
* by the Free Software Foundation; version 2.1 only. with the special
* exception on linking described in file LICENSE.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Lesser General Public License for more details.
*)

module Ipq = Xapi_stdext_threads_scheduler.Ipq

(* test we get "out of bound" exception calling Ipq.remove *)
let test_out_of_index () =
let q = Ipq.create 10 in
Ipq.add q {Ipq.ev= 123; Ipq.time= Mtime_clock.now ()} ;
let is_oob = function
| Invalid_argument s when String.ends_with ~suffix:" out of bounds" s ->
true
| _ ->
false
in
let oob_check n =
(Alcotest.match_raises "out of bound" is_oob @@ fun () -> Ipq.remove q n) ;
Alcotest.(check bool) "same value" false (Ipq.is_empty q)
in
oob_check 10 ;
oob_check (-1) ;
oob_check 9 ;
oob_check 1 ;
(* this should succeed *)
Ipq.remove q 0

let tests = [("test_out_of_index", `Quick, test_out_of_index)]

let () = Alcotest.run "Ipq" [("generic", tests)]
Empty file.

0 comments on commit cd98e9d

Please sign in to comment.