NoNewGlobals for SBufStats

[yadij]

Issues found by Coverity scan CID 1554550, 1554554, 1554574,
1554597, 1554604, 1554605, 1554613, 1554626, 1554602.

[rousskov]

Please apply NoNewGlobals code pattern rather than using a similar but different one:

Suggested change

	SBufStats &
	SBuf::Stats()
	{
	static auto *stats = new SBufStats;
	auto &
	SBuf::Stats()
	{
	static const auto stats = new SBufStats();

N.B. The same applies to virtually any methods in other PRs that can use that pattern. Reusing that pattern helps improve code quality and reduce review iterations/delays.

[rousskov]

We should not have two class methods named GetStats() and Stats(). The names should tell the two methods apart. This was discussed four days ago in another very similar PR: #1749 (comment). Let's reuse the same approach here:

• Use WriteableStats() name for the new method.
• Use GetStats() to preserve old method callers and use a simpler name for the safer method. This method should have been named Stats(), but we do not need to improve that in this PR.

Suggested change

	static const SBufStats & GetStats() { return Stats(); }
	static const auto &GetStats() { return WriteableStats(); }

If you prefer to also rename GetStats() to Stats() in this PR, I will support that change as well, but I do not insist on it. Your call.

[yadij]

IIRC the GetStats() only exists because you insisted on the single caller (mgr dump function) being provided a const. Statistics objects are generally always writable in Squid so they can be updated by the caller/user code, so calling it WritableStats is redundant.

[rousskov]

Alex: We should not have two class methods named GetStats() and Stats(). The names should tell the two methods apart. This was discussed four days ago in another very similar PR: #1749 (comment). Let's reuse the same approach here

Amos: IIRC the GetStats() only exists because you insisted on the single caller (mgr dump function) being provided a const. Statistics objects are generally always writable in Squid so they can be updated by the caller/user code, so calling it WritableStats is redundant.

The response quoted above clearly does not address my concern about PR code/changes.

I do not see "redundancy" in "[Get]Stats() and WriteableStats()" naming scheme. If you reject the proposed solution (mimicking the one developed in #1749), please propose something better than that solution. Hopefully, both PRs (and future PRs!) can use the same overall approach that we can agree on.