CURL review request

[Martin Nowak]

On Tue, 16 Aug 2011 20:48:51 +0200, jdrewsen <jdrewsen at nospam.com> wrote:

> Den 16-08-2011 18:55, Martin Nowak skrev:
>> On Tue, 16 Aug 2011 15:13:40 +0200, dsimcha <dsimcha at 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
>

Pardon the typo. What I meant is that AFAIK casting from immutable to  
mutable has undefined behavior.
The intended method for sending a uint[] buffer to another thread is to  
cast that
buffer to shared (cast(shared(uint[])) and casting away the shared on the  
receiving side.
It is allowed to send shared data using std.concurrency.

martin