[squid-dev] [MERGE] Fix splay

Amos Jeffries squid3 at treenet.co.nz
Fri Jan 2 11:24:41 UTC 2015


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2/01/2015 11:25 p.m., Kinkie wrote:
> Hi, splay uses lots of C-isms still, which make recent clang fail
> the build badly.
> 
> Things like: SplayNode<foo> *root = NULL root->insert(data)
> 
> The attached patch : - migrates all clients from using the
> now-private SplayNode API to Splay - fixes Splay to account for
> some corner-cases - removes always-true or always-false
> conditionals in SplayNode (if (this == NULL)) - ports parts of the
> unit test (most were not ported assuming the aim is to test the
> private API)
> 
> This builds and checks fine on ubuntu utopic with gcc and clang
> (clang halts the build on a similar issue in src/comm/Read.cc but
> that's another topic).
> 
> Public branch is at lp:~squid/squid/splayfix
> 
> To me this is a reasonable merge candidate; please review.
> 
> Thanks
> 


I believe this is missing several "delete Foo;" in the ACL
destructors. You used new() to allocate the Splay<> objects, there
should be matching delete() at the same owner/abstraction level.
For example:
"
 ACLIP::~ACLIP()
 {
    if (data) {
        data->destroy(IPSplay::DefaultFree);
        data->destroy();
        delete data;
     }
 }
"
... although, it would be even better if the data members could be
made non-pointers now they are Splay<> objects. Is that easy enough to
do in this patch ?


Please dont leave empty lines lying around. Like this one in
src/acl/Ip.h that was before the "private:"
"
     IPSplay *data;

- -private:
- -    static void DumpIpListWalkee(acl_ip_data * const & ip, void *state);
 };
"
 ... there are also empty lines being added like at the top of
include/splay.h:
"
 #include <stack>

+
+// private class of Splay. Do not use directly
"


in test-suite/splay.cc:
* s/oher/other/


Other than the new() / delete() issue. +1.

Amos
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (MingW32)

iQEcBAEBAgAGBQJUpn/5AAoJELJo5wb/XPRjHBEH/jSJmWgLn7hJNE11x22zxySy
zrlkcDt7MgFCID6bEDvJO27cmQIw+30yfx58pSYkQkyc5dI3xNmRrPyucUrBo6ih
zT11q8BhG/0I4XYVKuS6OhGxLwFIILqwEFL2szQ10XKgiJyhNaBbDXnrJZ+KFVOZ
srDgEwpGr3W1xp18riQX283VyX8n9vRpzGUa0GavHao8bqCGOuiKlGUc/aEXKMPz
S8aZ5eYBRhN6tuVhj41QpcdRBvmk/MfLS52v8EgH7+IBFT2gQyrkKikTERkqOCdn
tLDRDYoKAjavjrf7gdllXiZyrcMeBCyOSVjTWWrU48OYSK9UKrE+WfjiuxBCJJo=
=hxLW
-----END PGP SIGNATURE-----


More information about the squid-dev mailing list