RE: [PATCH] Fix memory leaks in ident.cc

On 27/12/2012 1:00:43 a.m., Amos Jeffries wrote: > Thank you. BUT... this adds a code desing regression in the form of an > Ident::Close_ static destructor function. > > Please make that a cleanup member method and simply call it when > appropriate. Including the state destructor. > > > PS. IdentStateData is long overdue for conversion from explicit > cbdataAlloc/cbdataFree memory management to use of the CBDATA_CLASS2() > constructs. Would you mind doing that conversion as part of this cleanup > since you can obviously test IDENT better than any of us has been able to? > The steps are simple: > * add CBDATA_CLASS2(IdentStateData) to the *end* of the IdentStateData > class definition > * add CBDATA_INIT(IdentStateData) to the top of the .cc file > * replace cbdataAlloc/cbdataFree of that class with new/delete. > * test that is still works without any new bugs (use of memset on the > objects is the main problem found here). > > Amos I really don't know what it is you want. I wrote my first C program in 1979 under Unix Version 6. This is the first C++ program I've really looked at and I find it incredibly obtuse. I spent two days staring at the code before I figured out state->conn = NULL; calls the destructor for the Connection object. To me the only thing object-oriented programming is good for is to allow poor programmers to rapidly develop slow programs. What is a "desing regression"? I looked at adding CBDATA_CLASS2 to the IdentStateData class definition but there isn't an IdentStateData class at all! There is an IdentConfig class, but not an IdentStateData class. I'm happy to test any patch you'd like, but I just don't understand what changes you want done to the existing patch. Someone much more familiar with C++ will have to help. Whatever the change is should also be done to dns_internal.cc. If its TCP connection fails it will leak a Connection object just like the original Ident code. Just look at idnsInitVCConnected() and you'll see it return without cleaning up. Mike Mitchell
Received on Fri Dec 28 2012 - 14:04:21 MST