Curl wrapper

Andrei Alexandrescu SeeWebsiteForEmail at erdani.org
Tue May 17 07:42:51 PDT 2011


Thanks for the response! A few more answers and comments within 
(everything deleted counts as "sounds great").

On 5/17/11 3:50 AM, Jonas Drewsen wrote:
>> 3. Data on wire is ubyte[], not void[].
>
> Data received is ubyte[] and a convenience method for conversion to
> string is provided.
>
> Data send is void[] since all arrays implicitely converts to void[].
> This makes e.g [1,2,3,4,5] works as input. Convenience methods are
> provided for chars in order to set HTTP headers correctly.
>
> For onSend callback it is simply void[] in order to use it as an out
> parameter.
> For Http.post/put it is const(void)[]
> For Http.postAsync/putAsync it is immutable(void)[] because internally I
> need to send the data to another thread using message passing.
>
> Hopes this makes sense?

I think it's best to systematically use void[] when sending data out 
(i.e. you start with structured data in memory and you let it implicitly 
"decay" as you pass it on the wire), and ubyte[] when receiving data 
(i.e. you start with unstructured data and force the user to explicitly 
build structure on it). As noted the current File API is not to be 
considered an example to follow.

>> 6. "Do not use the same instance of this class in two threads
>> simultanously." -> remove. A D program isn't supposed to do that unless
>> you offer shared methods.
>
> Ok. So even though I declare:
>
> shared Http http = new Http();
>
> I cannot have multiple threads call http.perform without that method
> being marked as shared as well?
>
> If thats the case I'll remove the comment.

Yes, shared limits access to only shared fields and methods.

>> 9. "this(in const(char)[] url);" -> "this(string url);" I assume you
>> store a copy of the URL inside by calling e.g. to!string. If the client
>> has a string, you refuse to tap into that nice information and share the
>> string safely and efficiently, and make a copy of it for no good reason.
>> If the client doesn't have a string, no worry, the compiler will tell
>> her she needs to do a copy.
>
> I do not store a copy but calls libcurl which in turn does a copy by
> itself. So I believe the current signature is appropriate?

Got it, thanks. I sense a little inefficiency here that we can't quite 
get rid of - the user passes a string, we must convert it to a 
zero-terminated char* (one allocation) and then libcurl copies it again 
(two allocations). Oh well.

>> 14. Isn't the max redirect configurable via a parameter?
>
> Yes. It is called Http.followLocation from libcurls followLocation
> option. I will rename it to maxRedirections for clearity.

You mention this about many higher-level functions: "Maximum 
redirections are set to 10." Then I'm thinking, if it's an important 
parameter, make it a defaulted parameter for the function.

>> 16. Documentation should point to descriptions of the HTTP methods
>> wrapped (e.g. "post", "del" etc).
>
> Do you mean point to the relevant RFC?

Yah, or a good tutorial. (I didn't know DEL existed!)

>> 22. byLine/byChunk should not expose a string or generally data that can
>> be shared safely by the client. That's because it would need to create a
>> new string with each iteration, which is very inefficient. Instead, they
>> should expose char[]/ubyte[] just like File.byLine does, and reuse the
>> buffer with each call. Essentially byLine/byChunk should be
>> near-zero-overhead means to transfer and inspect arbitrarily large
>> streams.
>
> byLine/byChunk is currenly only available for the async methods which is
> implemented using message passing. This means that for passing the
> message a copy has to be made because a message is by-value or immutable
> data. Is there another way for me to pass the message without doing a
> copy... some kind of move semantic?

A library element is planned but not on the short list yet. You can work 
around that by internally doing a cast. As long as you observe the 
commitment that buffers are not accessed simultaneously by more than one 
thread you're well within defined behavior.

One more suggestion - you may want to also provide classic boring 
synchronous read/write methods that take a user-provided buffer. I'm 
sure people could use such e.g. when they want to stream data inside 
their own threads, or even in the main thread if they don't mind blocking.

>> 24. The shutdown mechanism should be handled properly. Shutting down
>> libcurl would have all pending transfers instantly throw a special
>> exception. Without a shutdown API, applications won't be able to
>> implement e.g. a Quit button.
>
> If you're only using this API this should be handled. But I see that it
> is not documented.

Yah, in the future we need to put an onShutdown protocol inside core, 
similar to atexit() in C. Each library that may block would plant a 
hook. That way an application will be able to exit quickly and 
gracefully even if it has blocked threads.


Andrei


More information about the Digitalmars-d mailing list