On 08/24/2015 09:30 AM, Kinkie wrote: > the one case in the whole codebase where a wordlist element was > legitimately deleted.
This is not true. wordlistDestroy() deletes a wordlist object/element as well. You could (should?) have reimplemented wordlistDestroy() using the newly added wordlistChopHead() to reduce code duplication and friends declarations. > The code looks odd, but this is an unfortunate consequence of > asymmetric expectations on the construct/destruct side: construct > side requrires C++ semantics for MEMPROXY_CLASS, destruct side > callers expect C semantics. The assymetry cause is a little different IMHO: While constructing code creates a single element at a time, wordlistDestroy() callers want to destroy the whole list at once. > + // use wordlistDestroy instead > + ~wordlist() = default; I would add a "does not delete data members" comment to emphasize that we know what is going on and are content with it. Not important though. > +/** remove the first element in a wordlist, and return its key > + * > + * \note the returned key must be freed by the caller using safe_free > + * \note wl is altered so that it points to the second element > + * \return nullptr if pointed-to wordlist is nullptr. > + */ > +char *wordlistChopHead(wordlist **); > + We need to document that the first element is destroyed. For example, /** Remove and destroy the first element while * preserving and returning its key. ... I did not do a full audit and have no objections. Thank you, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev