[squid-dev] [PATCH] Replace new/delete operators using modern C++ rules
rousskov at measurement-factory.com
Fri May 20 20:11:53 UTC 2016
On 04/11/2016 09:16 AM, Alex Rousskov wrote:
> On 04/09/2016 10:50 PM, Amos Jeffries wrote:
>> In principle this is a great step forward, but I would like confirmation
>> about the portability side of things before it actually gets merged.
> Me too. [...]
> FWIW, the patch did compile fine on a few Linux boxes and one MacOS laptop.
* the existing code causes problems and violates C++ rules
* the fix does not have known problems
* the fix has been tested on several platforms
* nobody has committed to report results for untested platforms
* the Squid Project provides no access to those untested platforms
* testing this code is difficult so the set of volunteers is tiny
Given the above, I suggest to apply polished changes to trunk and see
what happens on those untested and hard-to-get-to platforms. Any
objections or better ideas?
> please keep in mind that beyond [unlikely] compilation
> failures, it would be difficult for non-developers to confirm that the
> patch does not break anything, especially without somebody writing
> detailed instructions on how to do that: AFAICT, the code in question
> does not affect any functionality that is both enabled by default and
> visible in Squid logs or responses.
> It would also be great to test whether the clang exclusion was motivated
> by the same outdated (or even wrong) reasons as the other hacks around
> this code -- we may be able to remove that last exclusion if those tests
> are successful. Again, this will require a person who knows how to read
> Valgrind reports and can test --enable-xmalloc-statistics effects.
More information about the squid-dev