[Libguestfs] [PATCH v2 2/8] curl: Change the API to use an abstract data type.

[Pino Toscano]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20160707/5ec55847/attachment.sig>