Re: apt https method v2
On Sat, 6 Apr 2002, Tomas Pospisek wrote:
> The https classes are now clean subclasses of the http ones - there's no
> trace of any SSL specific stuff in http any more.
No #ifdefs either. Yay
So.. lets see..
I'm not convinced that introducing the new Connection class instead of
changing the semantics of CircleBuf is really best. Seems to add alot of
extra text all over the place. If you look at carefully, the only reason
serverstate has the FD at all is so it can call select. It only needs the
FD for this, and it does not need the other baggage. The ugly cast inside
your dirved circlebufs is probably a fair indicator that something is
wrong. [see below for a more complete solution]
The makefile stuff is fine.
+ GetAccessParams(&Acquire, &DefaultPort, &ProxyStr);
This is C++.. references do exist. Good idea otherwise
This can't be right:
+ string DefProxy = _config->Find(Acquire);
+ string SpecificProxy = _config->Find(Acquire + ServerName.Host);
The 2nd should likely be:
+ string SpecificProxy = _config->Find(Acquire + "::" +ServerName.Host);
DefaultPort and ServerName.Access are at odds, they really need to
be matched:
+ if (Connect(Host,Port, ServerName.Access.c_str(), // Access == http[s]
+ DefaultPort, Connection->ServerFd,TimeOut,Owner) == false)
If a connection is ment to encapsulate the SSL session as well as it's
other gunk the this:
+ close(Connection->ServerFd);
Could read:
Connection->Close();
If you do that then at least the connection class is *doing* something and
not just acting as a crutch, you can eliminate the ServerStateSSL::Close
too.
Indeed, the overall size of the patch and the total additional code could
be considerably reduced if..
- Connection actually represented a conneciton and could do things with it
- CircleBuf was not subclassed
- Connection had connect, close, read and write methods
I'm confused why the connect_ssl.cc file exists at all. Suck it in the
https file with the rest of the SSL functions.
Comment at the top of https.cc should describe the interface to openssl.
Ugly:
+ Res = SSL_read(((ConnectionHandleSSL *)Connection)->Ssl,
+ Buf + (InP%Size),LeftRead());
How about Res = Connection->Read(..); and completely remove the duplicated
and virtual CircleBuf functions.
ServerState is created/destroyed many times during normal operation. I see
+ In = new CircleBuf(64*1024);
+ Out = new CircleBuf(4*1024);
+ Connection = new ConnectionHandle();
But no corrisponding deletes anywhere in the patch, thus it is leaking
memory.
> One other question - while hacking at apt, the many clog's were very
> use- and helpful to me to see what's happening "behind the scenes".
> Would you mind to keep them in if I'd replace them by s.th. like:
I prefer to remove them and use strace instead.
Jason
--
To UNSUBSCRIBE, email to deity-request@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Reply to: