Here are is a 2nd attempt at my option negotiation patch based on
Dan's comments. A few comments based on the new patch:

1.) If tftp_parse_option_ack() fails for any reason, I still chose to
return CURLE_TFTP_ILLEGAL rather than move on. I have done this
because if the server returns an option acknowledgement but we (the
client) fail to parse it and move on, the server may think we are
prepared to use a block size we are not ready for (the block size does
not get set until it is successfully parsed out of the OACK even
though the memory is allocated).

2.) It was mentioned that this function:

static int tftp_option_add(tftp_state_data_t *state, int csize, char
*buf, const char *option)
{
   if( ( strlen(option) + csize + 1 ) > state->blksize )
        return 0;
   strcpy(buf, option);
   return( strlen(option) + 1 );
}

could be simplified with Curl_strlcat(). I looked at the
Curl_strlcat() code and it requires that the destination buffer (buf
in this case) be NULL terminated and will not append more than the
length of the destination pointer - len - 1 bytes. At the time we
enter the function, buf should point to the character after the
current NULL terminator, the data beyond that is unknown as the buffer
may be re-used any number of times. As such, the string length of the
destination pointer will be variable. This is why I check that the
buffer location + the size of the new option is less than the
currently specified block size (the size allocated for the packet
buffer). At this point I just copy the string in, and return option
len + 1 (so successive calls point to the location in the buffer after
the current NULL terminator just added). I'm all for simplifying
things, but think the current approach would be the safest. If you
have an alternative simplified approach, I'd be curious to see it.

3.) I did not add a method to disable inclusion of options in the RRQ
or WRQ as I don't see a need for it right now. If anyone disagrees or
we see any issue, I have no problems adding it. One benefit of always
having them enabled is the progress call backs should now include the
proper dltotal value if the server side supports options.

Other than this, all requested changes should be included.

--
Chad Monroe