On Mon, 28 Jan 2013, Chrysovaladis Datsios wrote:

Thanks for efforts and contribution!

> +------------------------------+
> | documentation |
> +------------------------------+
>
> curl_easy_setopt() option: CURLOPT_HTTPTRAILERFUNCTION

I would prefer if we cut out the 'HTTP' from the name, and try to keep it
otherwise protocol agnostic. If we pretend this isn't HTTP specific, there's
no reason we can't use this callback for other protocols in another similar
scenario if things turn that way in a future.

> Pass a pointer to a function that matches the following prototype: int
> function(struct curl_slist *trailer_headers);

1 - you don't seem to document what the callback should return. It should at
the very least be able to return OK or ABORTEVERYTHING.

2 - your example code returns a pointer and not an 'int' as your description
says...

3 - all libcurl callbacks have an associated *DATA option that allows the user
to pass in a custom pointer to the callback. That's crucial for a callback to
be able to function without dreaded global variables.

> +/* pointer to function curl_trailerheaders_callback */
> +typedef struct curl_slist * (*curl_trailerheaders_callback)(CURL *handle,
> + struct curl_slist *trailer_headers);

4 - this doesn't match the documented prototype either!

> + /* If this is the last chunk */
> + if(data->req.upload_present == 5 &&
> + !strncmp(data->req.upload_fromhere, "0\r\n", 3) ) {

This seems like a very blunt way to detect the end of an upload. Surely this
information can be passed on/figured out in a better way!

> + data->set.trailerheaders_func = va_arg(param,
> + curl_trailerheaders_callback);
> + if(!data->set.trailerheaders_func)
> + data->set.is_trailerheaders_set = 0;
> + else
> + data->set.is_trailerheaders_set = 1;

What's the purpose of this boolean?

-- 
  / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html