On 04/10/2014 11:32 PM, Amos Jeffries wrote: > I believe the attached patch is complete regarding the method conversion.
The patch looks OK to me. The comments below are minor and can be ignored or fixed during commit. > + sb.append(s.rawContent(), s.length()); > + sb.append(s.rawContent(), s.length()); ... There are quite a few of such calls in the patch and they often require declaring a temporary SBuf variable. Consider adding MemBuf::append(const SBuf &sb) method to handle these better. > Log output quoting was what you meant. > + debugs(11, 2, "Request not yet fully sent \"" << request->method << > ' ' << entry->url() << '\"' ); > + debugs(11, 3, '\"' << fwd->request->method << ' ' << fwd->entry->url() > << '\"'); The above two changed lines still have unneeded (IMO) "log output quoting". > + logfilePrintf(logfile, "%s %s %s [%s] \"" SQUIDSBUFPH " %s %s/%d.%d\" %d > %" PRId64 " \"%s\" \"%s\" %s%s:%s%s", > clientip, > user_ident ? user_ident : dash_str, > user_auth ? user_auth : dash_str, > Time::FormatHttpd(squid_curtime), > - al->_private.method_str, > + SQUIDSBUFPRINT(method), I think it is only a matter of time when Time::FormatHttpd() or another function used in this or similar context will call SQUIDSBUFPRINT() to format something internally. This problem is out of your patch scope, of course, but we should move towards stream << printing to avoid such problems and resist adding more printfs. Cheers, Alex.
