[Libguestfs] [PATCH v2] OCaml tools: add and use a Getopt module

[Richard W.M. Jones]

On Mon, Jul 11, 2016 at 06:03:39PM +0200, Pino Toscano wrote:
> diff --git a/mllib/getopt-c.c b/mllib/getopt-c.c
> new file mode 100644
> index 0000000..e5e832c
> --- /dev/null
> +++ b/mllib/getopt-c.c
> +static void
> +do_call1 (value funv, value paramv)
> +{
> +  CAMLparam2 (funv, paramv);
> +  CAMLlocal1 (rv);
> +
> +  rv = caml_callback_exn (funv, paramv);
> +
> +  if (Is_exception_result (rv))
> +    fprintf (stderr,
> +             "libguestfs: uncaught OCaml exception in getopt callback: %s",

Does this need \n?

> +    case 0:
> +      if (STREQ (longopts[option_index].name, "help")) {
> +        show_help (specsv, usage_msgv);
> +      }
> +      /* specv_index set already -- nothing to do. */
> +      break;
> +
> +    case 'h':
> +      show_help (specsv, usage_msgv);
> +      break;

Is this right?  Several commands (eg. virt-df) take a -h option which
isn't for help.

The new code seems to be GC-safe as far as I can tell.

> +let parse_argv argv specs ?anon_fun usage_msg =
> +  (* Sanity check the input *)
> +  let validate_key key =
> +    if String.length key == 0 || key == "-" || key == "--"
> +       || key.[0] != '-' then
> +      raise (Invalid_argument (sprintf "invalid option key: '%s'" key))

Whereever you've written 'raise (Invalid_argument ...)' you can
replace it with 'invalid_arg ...'.

> +  let specs = specs @ [
> +    (* Handled internally by getopt_parse. *)
> +    [ "-h"; "-help"; "--help" ], Unit (fun () -> ()), s_"Display brief help";

As above.  Also I think it would be worth adding a check that we don't
have duplicate options in the list.

Personally I'd still like to see the L/S stuff, but this is certainly
a great improvement on what we have currently.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/