On Tue, Jan 06, 2009 at 05:33:54PM -0800, Chad Monroe wrote:
> 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).

Fair enough. It's probably reasonably safe.

> 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.

strlcpy may make more sense here, but this is a pretty minor point overall.

> 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.

It would be interesting to hear real operational experience from users of
RFC 2348 compliant TFTP clients. Are there any TFTP servers out there that
actually could be problematic? RFC 2348 is 10 years old so one would hope
not, but you never know.

>>> Dan

-- 
http://www.MoveAnnouncer.com              The web change of address service
          Let webmasters know that your web site has moved