I will answer in all-letter-covers-all style. :)

> In the header file, calling the first argument 'data' is a bit
> misleading, all other easy functions use 'curl'.

Agreed. :)

> Could the reqlen and n arguments be merged into one in/out argument?
> Like
> size_t size = sizeof(buf)
> CURLcode ret = curl_easy_send(curl, buf, &size);
> ?

Yes, of course.
But I would insist to keep them as two separate arguments.
If you forget to initialize the 'size' variable, the result will be
unpredictable: you may get random crashes, buffer overruns, etc. Keeping
a separate 'reqlen' argument *forces* you to specify the size.

> It would be nice if curl_easy_recv() returned a CURLcode, handling the
> possible -1 from Curl_read().

Yes, this was the initial draft design: in case there is nothing to
receive yet, curl_easy_recv() would return CURLE_RECV_ERROR.
But finally I thought it is not an error, but a special condition...
hmmm... it should be used in conjunction with some select() on the
socket...

Ok, I will report CURLE_RECV_ERROR.

> The second argument to curl_easy_send() should be a const char *.

Hmmm... Curl_write() uses "void*" for this parameter, so there will be a
cast... Or am I thinking C++?

> Curl_read() and Curl_write() expect a ssize_t pointer as last
argument,
> not size_t. Also if the public function are going to use size_t, you
> need to handle negative values set by Curl_{read,write}.

Hmmm...
Currently, both Curl_write() and Curl_read() report error in such
situations; anyway, it looks reasonable to double-check.

> You check for c == NULL a few lines after you set it, it would be more
> readable if you checked right after setting it.

Agreed.

> Why the "extra block for temporary ret1" in curl_easy_recv()?

Old habits. :)
Anyway, I will rewrite the code.

> One more: In both functions, 'ret' is declared in the middle of a
> compound statement, which requires C99, which is considered too new
for
> libcurl (AFAIK).

Aww... That's the evil influence of C++. Should be rewritten, of course.

> 1 - lack of docs (man pages)

From docs/CONTRIBUTE it is not quite clear how exactly the docs should
be written: "The documentation is always made in man pages (nroff
formatted) or plain ASCII files". So, man pages or ASCII? :)
I will take a man page as an example (say,
docs/libcurl/curl_easy_reset.3) and write the docs on its basis.

> 2 - I think the functions should better rely on a previous
CONNECT_ONLY
> call and return an error if that hasn't been the case. These funtions
> risk being able to attempt to do things even if that is not the case
and
> the resulting havoc will be weird and cause much confusion.

Well, in fact the risk is not that great: if there is no usable socket,
the functions will simply report error. The only way they can do some
confusion is when the connection is not closed after transfer; then,
indeed, writing arbitrary data can confuse the remote server.

Ok, I will add the check.

(Will post updated version in about an hour).
Received on 2008-04-29