Re: TFTP Option Negotiation
Date: Fri, 16 Jan 2009 14:45:33 -0800
On Jan 14, 2009, at 2:31 PM, Daniel Stenberg wrote:
> On Tue, 6 Jan 2009, Chad Monroe wrote:
>> Here are is a 2nd attempt at my option negotiation patch based on
>> Dan's comments. A few comments based on the new patch:
> Thanks. Over all it looks fine but there were just some minor flaws
> left, and at least one larger:
> - there were several source lines wider than 80 columns and some other
> indents that I disagreed with. I fixed them.
> - I got no less than 5 compiler warnings when I built (configure
> --enable-debug) so I modified a few data types from int to size_t
> and added
> a typecast to get rid of them.
> But neither your original version, nor my edit (see attachment) run
> through the test suite fine. 2003 seems to segfault and 2004 leaks
> memory... :-O
> Did you manage to run those tests fine?
> / daniel.haxx.se<tftp-3.patch>
Sorry about the compiler warnings, I didn't realize I needed to
configure with --enable-debug to get -Wall/-Werror CFLAGS.
I'm not sure how I missed test cases 2003 and 2004, but I installed
libssh2 so I could run them and can reproduce the same issues you see.
I know what is causing the problems, just not a good way to fix them.
For test case 2003, tftp_disconnect() is getting called after the
tftp_state_data_t struct is free'd I believe. If the state pointer is
not NULL, Curl_safefree() gets called for the data pointer in spacket
and rpacket. Tracing with GDB the value of these two pointers is
0x13... I looked around and see that the curl_dofree() function uses
memset() to set memory to this value before it releases it. Rather
than a NULL check, is there some other check I should be doing here?
The memory leak in test case 2004 is caused by the tftp_state_data_t
being released and re-allocated without calling tftp_disconnect(), it
appears to be happening when Curl_reset_reqproto() is called. Is
tftp_disconnect() not the right place to free my packet buffers? If
not, where would you recommend I do this? I basically need to free the
packet buffers any time the tftp_state_data_t struct is free'd. Thanks
again for the help,
-- Chad MonroeReceived on 2009-01-16