On 23/11/2013 5:35 a.m., Alex Rousskov wrote: > On 11/22/2013 08:25 AM, Amos Jeffries wrote: >> I've gone through the non-String code use of String::defined() and tried >> to find other ways of avoiding the defined()/NULL testing for ambiguous >> cases of String/SBuf differences. > > >> - } else if (rep->cache_control->hasNoCache() && >> !rep->cache_control->noCache().defined() && >> !REFRESH_OVERRIDE(ignore_must_revalidate)) { >> + } else if (rep->cache_control->hasNoCache() && >> rep->cache_control->noCache().size() > 0 && >> !REFRESH_OVERRIDE(ignore_must_revalidate)) { > > Bug. If you replace defined() with size(), this bug will not happen > because !defined() is going to be correctly replaced with !size(). > > >> - httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), >> &entry.detail); >> + int detailsParseOk = >> httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), &entry.detail); >> tmp = parser.getByName("descr"); >> - httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), >> &entry.descr); >> - bool parseOK = entry.descr.defined() && >> entry.detail.defined(); >> + int descrParseOk = >> httpHeaderParseQuotedString(tmp.termedBuf(), tmp.size(), &entry.descr); > > Please mark the new locals constant. > > > Throughout the patch please use size() instead of size()>0 if at all > possible, especially in the code that already uses succinct expressions > in this and/or other contexts. I cannot insist on that, of course, but I > have to ask. > > I did not find any other problems. > > > Thank you, > > Alex. >
Gah. This email got delayed and I comitted after you earlier/later one just saying done. Oh well, fixed the bugs here anyway. Amos