Re: Experimental PATCH (+ question): Problem with HTChannel_delete

On Sun, 3 Sep 2000, Karl-Otto Linn wrote:

> Date: Sun, 03 Sep 2000 19:06:53 +0200
> From: Karl-Otto Linn <linn@informatik.fh-wiesbaden.de>
> To: Peter Stamfest <peter.stamfest@eunet.at>
> Subject: Re: Experimental PATCH (+ question): Problem with
>     HTChannel_delete
> 
> Dear Peter,
> I think all people dealing with libbwww suffer the same problems: Nearly
> no
> docu, too little examples, no means to trace errors back to their real
> root
> and above all dead locks and access to previously freed memory regions.
> In your eample - and many others - I think you are right and it is up to
> "HTHost_setChannel(net->host, channel)"  to handle the fact that
> "channel"
> might be invalid/freed or NULL.

Well, I'm not sure what you mean by `...it is up to
"HTHost_setChannel(net->host, channel)"  to handle...' (or if there is a
`not' missing) because this is the opposite of what I mean.
HTHost_setChannel SHOULD NOT mind what I want to set the channel to,
because it CANNOT know what I want.

I think you are on my side, but I wanted to make it clear that the
"overprotective" style of libwww more often fires back than to do any
good.


> I have changed libwww (HTEvtLst.c) such that I am able to terminate the
> library in an somewhat regular way by pressing ESC (instead of ^C). So
> it is
> helpful for me to see your changes and ideas.

I'm currently trying to write a daemon using libwww. The good thing is,
that I have to deal with only a couple of different access patterns. If I
would have to deal with more than HTTP and FTP, I probably would give up
the whole thing.

Anyway, my FTP work seems nearly finished (at least the GET) part, now PUT
is waiting.... I forsee terrible things....


peter


> 
> mfg K.O.
> 
> 
> Peter Stamfest wrote:
> 
> > Please find attached an experimental patch that solves a HUGE problem for
> > me.
> >
> > It regards HTChannel_delete, which cowardly refuses to free (and
> > delete from hash tables) HTChannel objects with a socket set to
> > INVSOC. However, such a socket is set by libwww itself (in
> > HTDoClose). This causes HTChannel_deleteAll (which does not check for
> > "proper sockets") to really do the deletion of HTChannels. However, this
> > function is only called in HTLibTerminate (or a similar function), at a
> > point of time where many of the referenced objects have already been
> > freed, causing accesses to previously freed memory - a VERY BAD thing -
> > specially, as in my case, if the application has to continue to run...
> >
> > The attached patch corrects this problem - it allows to delete HTChannels
> > with a socket set to INVSOC, using the same hash-value strategy as
> > HTChannel_setSocket.
> >
> > I would like to add one question: Many of the *_setSomething routines in
> > the library not only check for a proper "me" pointer, but they also check
> > for non-NULL value pointers (if a pointer should be set). How is that
> > so? That way it gets impossible to break a link to an object. For
> > example: in HTNet.c:
> >
> > PUBLIC BOOL HTNet_setChannel (HTNet * net, HTChannel * channel)
> > {
> >     return (net && channel) ? HTHost_setChannel(net->host, channel) : NO;
> > }
> >
> > I think it should be
> >
> > PUBLIC BOOL HTNet_setChannel (HTNet * net, HTChannel * channel)
> > {
> >     return (net) ? HTHost_setChannel(net->host, channel) : NO;
> > }
> >
> > This would allow to properly unset the channel for the net. Similar
> > problems exist in many other parts of the library.
> >
> > Such "clever" routines always tend to complicate things, as for the ONE
> > time it is really needed to set some value of some object to NULL (because
> > a newly implemented protocol needs to do this, for example), the request
> > is not carried out. OK, the routine returns NO, but who would check this
> > (and react properly) when doing such a call [And, BTW, does the NO mean:
> > "you illegally passed a NULL pointer" or "HTHost_setChannel failed for
> > whatever reason" - The code handling such a situation has to check
> > for NULLness of the passed pointer to find THAT out - So if the
> > function-caller has to check it anyway, the caller can do it in
> > advance if it really is that forbidden. But at least this scheme
> > would allow to set a pointer to NULL if it is really, really
> > necessary.]? In my opinion such routines should not try to be
> > "clever" and to guess that the programmer did something wrong: It is not
> > up to a function to decide if a given argument is valid or not _in such
> > cases_.
> >
> > Any comments on this? Should we change this? I think we should, however
> > there is the old problem that those (IMHO) bugs might be used as features
> > in some code out there. I would call such code bogus, though.
> >
> > peter
> >
> >   ------------------------------------------------------------------------
> >                              Name: libwww-htchannel-patch
> >    libwww-htchannel-patch    Type: Plain Text (TEXT/PLAIN)
> >                          Encoding: BASE64
>

Received on Sunday, 3 September 2000 13:22:19 UTC