All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Matthew Booth <mbooth@redhat.com>, qemu-devel@nongnu.org
Cc: kwolf@redhat.com, peter.maydell@linaro.org
Subject: Re: [Qemu-devel] Curl updates
Date: Tue, 13 May 2014 13:47:18 -0600	[thread overview]
Message-ID: <537276C6.5010308@redhat.com> (raw)
In-Reply-To: <1399538540-5076-1-git-send-email-mbooth@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 2726 bytes --]

On 05/08/2014 02:42 AM, Matthew Booth wrote:
> [PATCH 1/4] curl: Fix parsing of readahead option from filename
> [PATCH 2/4] curl: Add sslverify option
> [PATCH 3/4] curl: Add usage documentation
> 
> The first 3 patches are reposted with updates following discussion of the option
> syntax. With this patch I've decided to break entirely with the previous syntax.
> Given that option parsing was previously both broken and undocumented, this is
> hopefully a forgivable sin.
> 
> The new syntax is:
> 
>   http://user:password@example.com/path?query[opt1=val:opt2=val]
> 
> I've bounded the option block in square brackets as these have no semantic
> meaning in any of the supported URI formats.

Offhand, I'm not liking this.  Why not use a completely valid URI, with
'.../path?query&opt1=val&opt2=val'?  Inventing your own
[opt1=val:opt2=val] on top of URI is asking for confusion.

Are you trying to support a way to pass a query string to the curl URI,
in addition to local options?  How often do curl URIs need a query?  Is
it something where you could use a local option named
'.../path?query=foo=bar' that contains anything to pass on to the raw
uri for curl as '.../path?foo=bar' (that is, ALL query name=value pairs
are local, but you have a name of 'query' whose value can be the
URI-encoded string to pass on as the name=value pairs for the raw URI
that you are passing through)?  That would be more consistent so that
the option is an actual URI to begin with.

> Consequently the user can escape
> them if they're unfortunate enough to have a URI which looks like it contains an
> option block.
> 
> I decided to separate options with colons rather than commas because commas play
> havoc with qemu's command line parsing. There's presumably a way round this, but
> I couldn't guess it and I was too lazy to look it up, so I assume users would
> feel the same.

Using ',,' behaves as an escape for any literal comma, when doing qemu
command line option parsing.

> 
> As options are now unambigous, invalid options now result in an error.
> 
> [PATCH 4/4] curl: Fix build when curl_multi_socket_action isn't
> 
> The last patch is unrelated. It should fix build against old curl, although I
> don't have an old curl kicking around to test it against.
> 
> I still have a couple of patches in my local tree which:
> 
> * Remove blocking behaviour in curl_open
> * Don't send EIO when a read connection hits a timeout
> 
> I'm also planning to add another option for timeout length, and to implement
> write support.
> 
> Matt
> 
> 
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  parent reply	other threads:[~2014-05-13 19:47 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-08  8:42 [Qemu-devel] Curl updates Matthew Booth
2014-05-08  8:42 ` [Qemu-devel] [PATCH 1/4] curl: Fix parsing of readahead option from filename Matthew Booth
2014-05-13 17:29   ` Eric Blake
2014-05-14 16:00     ` Matthew Booth
2014-05-14 16:55       ` Eric Blake
2014-05-08  8:42 ` [Qemu-devel] [PATCH 2/4] curl: Add sslverify option Matthew Booth
2014-05-08  8:42 ` [Qemu-devel] [PATCH 3/4] curl: Add usage documentation Matthew Booth
2014-05-08  8:42 ` [Qemu-devel] [PATCH 4/4] curl: Fix build when curl_multi_socket_action isn't available Matthew Booth
2014-05-13 19:47 ` Eric Blake [this message]
2014-05-14  7:48   ` [Qemu-devel] Curl updates Kevin Wolf
2014-05-14 12:59     ` Eric Blake
2014-05-14 16:08     ` Matthew Booth
2014-05-14 16:43       ` Kevin Wolf
2014-05-14 21:20         ` Matthew Booth
2014-05-14 21:36           ` Eric Blake
2014-05-14 16:59       ` Eric Blake
2014-05-14 16:06   ` Matthew Booth
2014-05-14 17:02     ` Eric Blake
2014-05-14 20:45       ` Matthew Booth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=537276C6.5010308@redhat.com \
    --to=eblake@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mbooth@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.