Re: [PATCH v2 1/1] tools/ocaml: Fix oxenstored build warning

[Andrii Sultanov <andrii.sultanov@xxxxxxxxx>]

> What's this hunk for?  There's a change in poll.ml, but I don't see why
> it would need to change this list.

Otherwise Poll doesn't pick up Utils as its dependency - I guess before it was always independent and didn't need anything like that

On Fri, Feb 14, 2025 at 3:42 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:

On 14/02/2025 3:24 pm, Andrii Sultanov wrote:
> OCaml, in preparation for a renaming of the error string associated with
> conversion failure in 'int_of_string' functions, started to issue this
> warning:
> ```
> File "process.ml", line 440, characters 13-28:
> 440 |   | (Failure "int_of_string")    -> reply_error "EINVAL"
>                    ^^^^^^^^^^^^^^^
> Warning 52 [fragile-literal-pattern]: 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 11.5)
> ```
>
> Deal with this at the source, and instead create our own stable
> ConversionFailure exception that's raised on the None case in
> 'int_of_string_opt'.
>
> 'c_int_of_string' is safe and does not raise such exceptions.
>
> Signed-off-by: Andrii Sultanov <andrii.sultanov@xxxxxxxxx>
> Acked-by: Christian Lindig <christian.lindig@xxxxxxxxx>
> ---
> Changes since v1:
> * Revert logging added to error handling in process.ml, return just "EINVAL"

Thanks.  This looks better.  One quick question.

> ---
>  tools/ocaml/xenstored/Makefile     |  1 +
>  tools/ocaml/xenstored/perms.ml     |  2 +-
>  tools/ocaml/xenstored/poll.ml      |  2 +-
>  tools/ocaml/xenstored/process.ml   | 18 +++++++++---------
>  tools/ocaml/xenstored/utils.ml     | 10 ++++++++--
>  tools/ocaml/xenstored/xenstored.ml | 16 ++++++++--------
>  6 files changed, 28 insertions(+), 21 deletions(-)
>
> diff --git a/tools/ocaml/xenstored/Makefile b/tools/ocaml/xenstored/Makefile
> index 5e8210a906..c333394a34 100644
> --- a/tools/ocaml/xenstored/Makefile
> +++ b/tools/ocaml/xenstored/Makefile
> @@ -54,6 +54,7 @@ OBJS = paths \
>       history \
>       parse_arg \
>       process \
> +     poll \
>       xenstored
> 

What's this hunk for?  There's a change in poll.ml, but I don't see why
it would need to change this list.

~Andrew