[squid-dev] [PATCH] SBuf API improvements

[Alex Rousskov]

On 12/28/2015 06:41 AM, Kinkie wrote:

>   the attached patch is a proposal for a couple of small extensions to
> SBuf, which are meant to make it more API-compatible with std::string.

... as well as increase the probability of callers using the wrong SBuf
API as discussed below.


> - it adds a "fill" operator: append(size_type n, char c)
> - it aliases append(char) to push_back(char)

If this change goes forward, I recommend removing our custom
append(char) because it conflicts with the other "standard" append,
increasing API confusion:

  string& append(const char* s);

The old append(char) calling code would need to be adjusted to call
push_back(), of course.



> - it aliases reserveCapacity to reserve

The standard reserve() has a default argument value of zero. Yours does
not. If you want compatibility, you have to provide that default IMO.


> It is debateable whether reserveCapacity should be just renamed
> instead of aliased; feedback is welcome.

I do not have a strong opinion on this. reserveCapacity() is not called
reserve() today because we also have reserveSpace() and knowing that
there are two different methods is important for some callers. Once you
add plain reserve(), the probability that a caller will notice
reserveSpace() that the caller needs will go down.


> The objective of this change will be clear in the followup rfc3986
> patch, which is templatized in order to be available both to helpers
> who use std::string and to squid using SBuf.

SBuf is already suffering from being too many things to too many
different callers. I suspect that requiring API compatibility with
std::string [in certain poorly defined areas] will make the situation
worse, but I cannot prove that. Same for writing templated code that can
work with both std::string and SBuf.

If helpers really need SBuf, give them SBuf. If helpers want to use
std::string, they already can. Assuring that one can be substituted with
another feels like a false goal that would bring more trouble in the
long run than it would help in the short term. Again, I cannot prove
that and will not veto these changes.


HTH,

Alex.