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.
