This is looking much cleaner than before! I've spotted a few issues
left, though.

On Thu, Feb 28, 2013 at 12:00:51PM +0200, Chrysovaladis Datsios wrote:
> @@ -193,8 +194,66 @@ CURLcode Curl_fillreadbuffer(struct connectdata *conn, int bytes, int *nreadp)
> /* copy the prefix to the buffer, leaving out the NUL */
> memcpy(data->req.upload_fromhere, hexbuffer, hexlen);
>
> + /* If is the last chunk, here put the trailer headers if any */
> + if((nread - hexlen) == 0) {
> + if(data->set.trailerheaders_func) {
> + struct curl_slist *trailer_headers;
> + struct curl_slist *list_head;
> +
> + /* The callback function that adds trailer header values */
> + trailer_headers = (data->set.trailerheaders_func)(data,
> + data->set.trailer_udata);
> + list_head = trailer_headers;
> + if(trailer_headers) {
> + char *ptr;
> + size_t headers_buf_size;
> + size_t endl_len;
> + size_t data_len;
> + size_t headers_index;
> + char *trailer_headers_buf;
> +
> + endl_len = strlen(endofline_network);
> + headers_buf_size = HEADERSIZE;
> + trailer_headers_buf = malloc(headers_buf_size);
> + if(trailer_headers_buf == NULL)
> + return CURLE_OUT_OF_MEMORY;
> + memcpy(trailer_headers_buf, hexbuffer, hexlen);
> + headers_index = hexlen;
> +
> + while(trailer_headers) {
> + /* header name_field : header value_field */
> + ptr = strchr(trailer_headers->data, ':');
> + if(ptr) {
> + data_len = strlen(trailer_headers->data);
> + if(headers_index + data_len + 3*endl_len > headers_buf_size) {

Where does the magic number 3 come from?

> + char *newbuf;
> + size_t newsize = headers_index + data_len + 3*endl_len;
> + if(newsize > CURL_MAX_HTTP_HEADER)
> + return CURLE_OUT_OF_MEMORY;

This will leak trailer_headers_buf. And I'm not sure CURLE_OUT_OF_MEMORY is the
best error code here--the system is technically not out of memory. Maybe
CURLE_BAD_FUNCTION_ARGUMENT?

> + newbuf = realloc(trailer_headers_buf, newsize);
> + if(newbuf == NULL)
> + return CURLE_OUT_OF_MEMORY;

This will leak trailer_headers_buf.

> + headers_buf_size = newsize;
> + trailer_headers_buf = newbuf;
> + }
> + memcpy(trailer_headers_buf + headers_index,
> + trailer_headers->data, data_len);
> + headers_index += data_len;
> + memcpy(trailer_headers_buf + headers_index,
> + endofline_network, endl_len);

These headers have to be sent through the CURL_DOES_CONVERSIONS code if
enabled, which means you want endofline_native here instead.

> + headers_index += endl_len;
> + }
> + trailer_headers = trailer_headers->next;
> + }
> + trlen = (int)(headers_index - hexlen);
> + data->req.upload_fromhere = trailer_headers_buf;

I don't think there is any code that frees this pointer anywhere, which means
it will leak.

> + curl_slist_free_all(list_head);

I'm not sure it's the best to free this list here. The caller may want to
keep it around, and it can always free it if it wants.

> + }
> + }
> + }
> +
> /* always append ASCII CRLF to the data */
> - memcpy(data->req.upload_fromhere + nread,
> + memcpy(data->req.upload_fromhere + nread + trlen,

Why not just increase the value of nread? Adding trlen everywhere complicates
the code, and makes it ease to forget to add it. Such as in the
CURL_DOES_CONVERSIONS section (which breaks because it was forgotten).

> endofline_network,
> strlen(endofline_network));
>
> @@ -231,7 +290,7 @@ CURLcode Curl_fillreadbuffer(struct connectdata *conn, int bytes, int *nreadp)
> }
> #endif /* CURL_DOES_CONVERSIONS */

The CURL_DOES_CONVERSIONS section has not been updated by this patch and will
break when enabled, due to converting the wrong data.

>
> - *nreadp = nread;
> + *nreadp = nread + trlen;
>
> return CURLE_OK;
> }

[...]

> int main(void)
> {
> CURL *curl;
> CURLcode res;
> FILE *fd;
> struct curl_slist *custom_http_hdrs = NULL;
>
> fd = fopen("video_0000", "rb");
> if(!fd)
> return 1;
>
> curl = curl_easy_init();
> if(curl) {
>
> curl_easy_setopt(curl, CURLOPT_URL, "http://10.8.6.65/myfile");

I'd use a more generic URL here.

>
> curl_easy_setopt(curl, CURLOPT_UPLOAD, 1L);
> curl_easy_setopt(curl, CURLOPT_READDATA, fd);
> curl_easy_setopt(curl, CURLOPT_READFUNCTION, read_callback);
>
> custom_http_hdrs = curl_slist_append(custom_http_hdrs,
> "Transfer-Encoding: chunked");
> custom_http_hdrs = curl_slist_append(custom_http_hdrs,
> "Trailer: mytrailer");
> curl_easy_setopt(curl, CURLOPT_HTTPHEADER, custom_http_hdrs);
>
> curl_easy_setopt(curl, CURLOPT_TRAILERFUNCTION, trailerheader_callback);
> curl_easy_setopt(curl, CURLOPT_TRAILERDATA, &filecode);
>
> res = curl_easy_perform(curl);
> if(res != CURLE_OK) {
> fprintf(stderr, "curl_easy_perform() failed: %s\n",
> curl_easy_strerror(res));
> }
>
> curl_slist_free_all(custom_http_hdrs);
> curl_easy_cleanup(curl);
> }
> fclose(fd);
>
> return 0;
> }
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-02-28