On 11/01/2016 02:02 PM, Kinkie wrote: > the attached patch extends SBufContainerJoin to have prefix and > suffix arguments. This can support a use-case which I found in the > current ACLRegexData work I'm following, where we need to transform > {"foo", "bar", "gazonk"} > into > (foo)|(bar)|(gazonk) > > It can be done with the current version of the SBufContainerJoin > function, but it requires one more copy. > There are probably further cases of this pattern elsewhere, all hand-rolled.
I agree that the use case is legitimate, but the implementation does not solve the underlying cause -- extra copies. I recommend reworking this by adding a dest parameter: /// copies all container items (delimited by separator) into dest ... /// \returns dest SBuf &ContainerToSBuf(SBuf &dest, ...); and using that function to implement the SBufContainerJoin() or a similarly named convenience wrapper that does not require a dest parameter and that returns a copy of an internally used SBuf. In either case, please do not forget to document that the new prefix and suffix are always added, even if the container is empty. > // sz can be zero in two cases: ... or all items are zero-length. FWIW, this is a lie. Zero-length items do not result in zero sz unless the separator itself is also surprisingly empty. Zero-length items is not a special case worth mentioning IMO. The zero-size check should be replaced with a direct (begin != end) check to avoid complexity and confusion -- it is only needed to avoid dereferencing begin() AFAICT. HTH, Alex. _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev