Re: PATCH: Much improved FTP support!!

On Fri, 1 Sep 2000 jose.kahan@w3.org wrote:

Jose, 

I have fixed the "problems" that you saw, and I send you the next version
of my patch. (Sorry for the many versions, but I keep fixing things in the
FTP code, my turn around time is much smaller than any applying to CVS
could ever be).

This patch applies using patch -p 1 in the top level directory of the
libwww distribution. 

It contains some changes not in my previous patch, that makes FTP much
more stable for me. (I was able to complete a mixture of 10000 HTTP and
FTP GET requests without any problems/memory leaks).

The patch contains some new things:
+ A fix in HTChannel_delete that fixes all my memory related problems I
  have reported [ I have sent this small patch to the list already, marked
  as experimental ]

+ Much revamped handling of the data connection of a FTP transfer. This
  might be taken as a starting point to clean up FTP code.

+ A change in the order of calling delete function in HTLibTerminate. At
  least one of functions  called after the HTAtom_deleteAll function used
  to use Atoms by itself, thus reallocating some Atom related space.


I hope this will be the last version of the patch before it goes into
CVS. I hope you can apply on Monday.

peter

> Date: Fri, 1 Sep 2000 16:40:01 +0200 (MET DST)
> From: jose.kahan@w3.org
> To: Peter Stamfest <peter.stamfest@eunet.at>
> Subject: Re: PATCH: Much improved FTP support!!
> 
> Peter,
> 
> I haven't yet commited them, as I saw some problems.
> 
> ============= HTChannel
> 
> I don't like that HTChannel_delete() also deletes the protocol context. I
> saw from your comments that you were also looking at this. 
> 
> What I suggest as a change is that you modify 
> HTChannel_setProtocolContext() so that you can also register the function that
> will delete the context. Then, in HTChannel_delete(), you can call this
> function and let it do whatever is necessary.
> 
> Does this make sense to you?
> 
> 
> ====== initialization
> 
> The other is just a minor remark. When you're initializing the 
> ctx->uid and ctx->passwd variables in HTFTP.c:1574-1575, you should
> initialize them to '\0' rather than 0, as their are characters.
>  
> ====
> 
> Also, I see you have a DUMPBLOCK function and some calls to it here and
> there. If you find it useful to have this function there while debugging,
> it may be better to make a define at top of the file and protect your
> blocks and function calls around this define. It'll make it easier to
> detect and remove... or reactivate. Something in the order of:
> 
> #define LIBWWW_FTP_DEBUG
> 
> ======== HTHost_new_with_check
> 
> The function's comment seems incomplete. It ends with "to test".
> 
> ==== HTHstMan.html
> 
> You defined a protocolContext field, but it doesn't seem to be used
> anywhere else.
> 
> Is this patch obsolete now?
> 
> ===========
> 
> Don't forget to sign your comments (crude something) so that we know
> from where they came :) Just put your initials and the date.
> 
> I'm now compiling the code and will test it. The HTHost changes look
> a bit complicated, so I want to make sure that they don't break something
> else.
> 
> If I don't notice anything strange, I'll commit your code on Monday, 
> provided you send me the changes I told you some time later (or discuss
> them).
> 
> Note that I'm away from office during 10 days, so I may have some delay in 
> answering you.
> 
> 
> -Jose
>

Received on Monday, 4 September 2000 01:03:11 UTC