Do not rely on the string values of the `Failure` exception,
 but use the `_opt` functions instead.

Signed-off-by: Edwin Török <edwin.torok@cloud.com>
---
 tools/ocaml/xenstored/config.ml | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/tools/ocaml/xenstored/config.ml b/tools/ocaml/xenstored/config.ml
index 95ef745a54..e0df236f73 100644
--- a/tools/ocaml/xenstored/config.ml
+++ b/tools/ocaml/xenstored/config.ml
@@ -83,25 +83,27 @@ let validate cf expected other =
   let err = ref [] in
   let append x = err := x :: !err in
   List.iter (fun (k, v) ->
+      let parse ~err_msg parser v f =
+        match parser v with
+        | None -> append (k, err_msg)
+        | Some r -> f r
+      in
       try
         if not (List.mem_assoc k expected) then
           other k v
         else let ty = List.assoc k expected in
           match ty with
           | Unit f       -> f ()
-          | Bool f       -> f (bool_of_string v)
+          | Bool f       -> parse ~err_msg:"expect bool arg" bool_of_string_opt v f
           | String f     -> f v
-          | Int f        -> f (int_of_string v)
-          | Float f      -> f (float_of_string v)
-          | Set_bool r   -> r := (bool_of_string v)
+          | Int f        -> parse ~err_msg:"expect int arg" int_of_string_opt v f
+          | Float f      -> parse ~err_msg:"expect float arg" float_of_string_opt v f
+          | Set_bool r   -> parse ~err_msg:"expect bool arg" bool_of_string_opt v (fun x -> r := x)
           | Set_string r -> r := v
-          | Set_int r    -> r := int_of_string v
-          | Set_float r  -> r := (float_of_string v)
+          | Set_int r    -> parse ~err_msg:"expect int arg" int_of_string_opt v (fun x -> r:= x)
+          | Set_float r  -> parse ~err_msg:"expect float arg" float_of_string_opt v (fun x -> r := x)
       with
       | Not_found                 -> append (k, "unknown key")
-      | Failure "int_of_string"   -> append (k, "expect int arg")
-      | Failure "bool_of_string"  -> append (k, "expect bool arg")
-      | Failure "float_of_string" -> append (k, "expect float arg")
       | exn                       -> append (k, Printexc.to_string exn)
     ) cf;
   if !err != [] then raise (Error !err)
-- 
2.44.0

> On 27 Mar 2024, at 16:30, Edwin Török <edwin.torok@cloud.com> wrote:
> 
> Do not rely on the string values of the `Failure` exception,
> but use the `_opt` functions instead.
> 
> Signed-off-by: Edwin Török <edwin.torok@cloud.com>
> ---
> tools/ocaml/xenstored/config.ml | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/ocaml/xenstored/config.ml b/tools/ocaml/xenstored/config.ml
> index 95ef745a54..e0df236f73 100644
> --- a/tools/ocaml/xenstored/config.ml
> +++ b/tools/ocaml/xenstored/config.ml
> @@ -83,25 +83,27 @@ let validate cf expected other =
>   let err = ref [] in
>   let append x = err := x :: !err in
>   List.iter (fun (k, v) ->
> +      let parse ~err_msg parser v f =
> +        match parser v with
> +        | None -> append (k, err_msg)
> +        | Some r -> f r
> +      in
>       try
>         if not (List.mem_assoc k expected) then
>           other k v
>         else let ty = List.assoc k expected in
>           match ty with
>           | Unit f       -> f ()
> -          | Bool f       -> f (bool_of_string v)
> +          | Bool f       -> parse ~err_msg:"expect bool arg" bool_of_string_opt v f
>           | String f     -> f v
> -          | Int f        -> f (int_of_string v)
> -          | Float f      -> f (float_of_string v)
> -          | Set_bool r   -> r := (bool_of_string v)
> +          | Int f        -> parse ~err_msg:"expect int arg" int_of_string_opt v f
> +          | Float f      -> parse ~err_msg:"expect float arg" float_of_string_opt v f
> +          | Set_bool r   -> parse ~err_msg:"expect bool arg" bool_of_string_opt v (fun x -> r := x)
>           | Set_string r -> r := v
> -          | Set_int r    -> r := int_of_string v
> -          | Set_float r  -> r := (float_of_string v)
> +          | Set_int r    -> parse ~err_msg:"expect int arg" int_of_string_opt v (fun x -> r:= x)
> +          | Set_float r  -> parse ~err_msg:"expect float arg" float_of_string_opt v (fun x -> r := x)
>       with
>       | Not_found                 -> append (k, "unknown key")
> -      | Failure "int_of_string"   -> append (k, "expect int arg")
> -      | Failure "bool_of_string"  -> append (k, "expect bool arg")
> -      | Failure "float_of_string" -> append (k, "expect float arg")
>       | exn                       -> append (k, Printexc.to_string exn)
>     ) cf;
>   if !err != [] then raise (Error !err)
> -- 
> 2.44.0
> 


Acked-by: Christian Lindig <christian.lindig@cloud.com>

On 27/03/2024 4:30 pm, Edwin Török wrote:
> Do not rely on the string values of the `Failure` exception,
>  but use the `_opt` functions instead.
>
> Signed-off-by: Edwin Török <edwin.torok@cloud.com>

FWIW, Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>

But I think the subject wants to say "in config.ml" and the commit
message gain something like:

Fixes warnings such as:

  File "config.ml", line 102, characters 12-27:
  102 |         | Failure "int_of_string"   -> append (k, "expect int arg")
                    ^^^^^^^^^^^^^^^
  Warning 52: Code should not depend on the actual values of
  this constructor's arguments. They are only for information
  and may change in future versions. (See manual section 9.5)

I can adjust all on commit.

~Andrew