Den 16-08-2011 18:55, Martin Nowak skrev:
On Tue, 16 Aug 2011 15:13:40 +0200, dsimcha <dsim...@yahoo.com> wrote:
On 8/16/2011 7:48 AM, Jonas Drewsen wrote:
Hi all,
This is a review request for the curl wrapper. Please read the "known
issues" in the top of the source file and if possible suggest a
solution.
We also need somebody for running the review process. Anyone?
Code:
https://github.com/jcd/phobos/blob/curl-wrapper/etc/curl.d
Docs:
http://freeze.steamwinter.com/D/web/phobos/etc_curl.html
Demolish!
/Jonas
From a quick look, this looks very well thought out. I'll review it
more thoroughly when I have more time. A few questions/comments from a
quick look at the docs:
Does the async stuff use D threads, or does Curl have its own async API?
In your examples for postData, you have onReceive a ubyte[] and write
it out to console. Did you mean to cast this to some kind of string?
For onReceive, what's the purpose of the return value?
If/when this module makes it into Phobos, are we going to start
including a libcurl binary with DMD distributions so that std.curl
feels truly **standard** and requires zero extra configuration?
I was also wondering about the async handling. In the long-term I'd like
to see a bigger picture for async handling in phobos (offering some kind
of futures, maybe event-loops etc.).
Though this is not a requirement for the curl wrapper now.
std.parallelism also has some kind of this stuff and file reading would
benefit from it too.
This has been discussed before and I also think this is very important.
But before that I think some kind of package management should be
prioritized (A DIP11 implementaion or a more traditional solution).
One thing I spotted at a quick glance, sending to be filled buffers to
another thread should not be done by casting to shared not immutable.
I'm not entirely sure what you mean. There is no use of shared buffers
in the wrapper. I do cast the buffer between mutable/immutable because
only immutable or by value data can be passed using std.concurrency.
Since the buffers are only used by the thread that currently has the
buffer this is safe. I've previously asked for a non-cast solution (ie.
some kind of move between threads semantic for std.concurrency) but was
advised that this was the way to do it.
martin
