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

Reply via email to