Hi all, it's been 13 days since the last comment on this thread. May I assume go-ahead for commit?
On Fri, Nov 18, 2011 at 8:42 AM, Kinkie <[email protected]> wrote: >>> +/** >>> + * look for the last occurrence of a character in a c-string with a set >>> maximum length >>> + */ >>> +SQUIDCEXTERN const char *strnrchr(const char *s, size_t slen, char c); >> >> Please fix the strnrchr() description. To match the implementation, the >> description should say that we scan from the beginning and stop at the >> end of the c-string or n-th character, whichever comes first (and >> s/slen/n/). > > Done (although s/slen/count/ to match the > >> I would also recommend checking whether your strnrchr() implementation >> matches that of GNU API. Their documentation is even worse: >> http://gnugeneration.com/mirrors/kernel-api/r2320.html > > Done. > >> >> >>> - if (!sctusable || sctusable->content.size() == 0) >>> + if (!sctusable || sctusable->hasContent()) >> >> Sorry if I asked you about this already, but is the reversal of the >> second condition above intentional? > > No, it wasn't. > Thanks for spotting that, and sorry for missing it if you asked already :( > >>> class HttpHdrSc >>> { >>> >>> public: >>> + bool parse(const String *str); >>> + ~HttpHdrSc(); >>> + HttpHdrSc(const HttpHdrSc &); >> >> Please move the parse() declaration below constructors/destructors. > > Done. > >>> + HttpHdrSc() {}; >> >> Extra semicolon. > > Removed. > >>>>> >> + String Content() const { return content; } >>>> > >>>> > I would s/content/content_/ instead, especially since content data >>>> > member is private. Capitalization should be used for static methods. >>> >>> Ok. >> >> If that "Ok" meant "will do later", then please note that you have not >> done that. > > Gah! Sorry.. Now done. > New version attached. > > -- > /kinkie -- /kinkie
