On Thursday 07 July 2016 16:22:05 Richard W.M. Jones wrote: > Change the Curl module to use an ADT to store the name of the curl > binary and the arguments.
Good idea.
The implementation looks mostly good, although I have a couple of notes.
>
> Also add Curl.safe_args, a list of arguments that control redirects etc.
The current implementation basically "forces" every caller to pass them
to Curl.create (and indeed this patch and patch #3 do this job), which
IMHO makes it easy to forget them.
Since we want those arguments on by default in all the curl invocations,
I'd do the following: hide Curl.safe_args, and add them automatically
in Curl.to_string. If in the future there will be the need to not use
them for some curl invocation, we can always add ~use_safe_args:false
for Curl.create.
> +type proxy =
> + | UnsetProxy (** The proxy is forced off. *)
> + | ForcedProxy of string (** The proxy is forced to the specified URL. *)
> +
> +val args_of_proxy : proxy -> args
> +(** Convert the proxy setting to the equivalent list of curl arguments.
> +
> + To use the system proxy, no additional arguments are required, so
> + if you don't want to control the proxy (but just use the defaults)
> + you do not need to call this function at all.
> +
> + Callers should append these arguments to the list of arguments
> + passed to {!create}. *)
Just wondering whether it makes sense a method to set the proxy for a
Curl.t handle, letting Curl do the proxy -> args conversion internally
(less work for users).
Thanks,
--
Pino Toscano
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
