On 19/04/2014 9:04 a.m., Alex Rousskov wrote:
> On 04/18/2014 12:12 PM, Amos Jeffries wrote:
>
>> If you are okay with the new bits marked (EXHIBIT A) and (EXHIBIT B) I
>> think we are done. These have already been checked against all unit
>> tests and passed.
>
> Please repost the current code/patch. Since most bugs were in the
> dependencies between different code parts, I do not think it is a good
> idea to review isolated EXHIBITs. We already know that you have not
> decreased the code complexity, so I am not going to _like_ what I see,
> but that is not the acceptance criteria either.
>
Fair enough. Attached.
Amos
=== modified file 'src/SBuf.cc'
--- src/SBuf.cc 2014-04-06 07:08:04 +0000
+++ src/SBuf.cc 2014-04-18 18:11:27 +0000
@@ -360,64 +360,116 @@
store_->mem[off_+pos] = toset;
++stats.setChar;
}
static int
memcasecmp(const char *b1, const char *b2, SBuf::size_type len)
{
int rv=0;
while (len > 0) {
rv = tolower(*b1)-tolower(*b2);
if (rv != 0)
return rv;
++b1;
++b2;
--len;
}
return rv;
}
int
-SBuf::compare(const SBuf &S, SBufCaseSensitive isCaseSensitive, size_type n)
const
+SBuf::compare(const SBuf &S, const SBufCaseSensitive isCaseSensitive, const
size_type n) const
{
if (n != npos)
return substr(0,n).compare(S.substr(0,n),isCaseSensitive);
- size_type byteCompareLen = min(S.length(), length());
+ const size_type byteCompareLen = min(S.length(), length());
++stats.compareSlow;
int rv = 0;
if (isCaseSensitive == caseSensitive) {
rv = memcmp(buf(), S.buf(), byteCompareLen);
} else {
rv = memcasecmp(buf(), S.buf(), byteCompareLen);
}
if (rv != 0)
return rv;
if (length() == S.length())
return 0;
if (length() > S.length())
return 1;
return -1;
}
+int
+SBuf::compare(const char *s, const SBufCaseSensitive isCaseSensitive, const
size_type n) const
+{
+ // 0-length comparison is always true regardless of buffer states
+ if (!n) {
+ ++stats.compareFast;
+ return 0;
+ }
+
+ // N-length compare MUST provide a non-NULL C-string pointer
+ assert(s);
+
+ // when this is a 0-length string, no need for any complexity.
+ if (!length()) {
+ ++stats.compareFast;
+ return '\0' - *s;
+ }
+
+ // brute-force scan in order to avoid ever needing strlen() on a c-string.
+ ++stats.compareSlow;
+ const char *left = buf();
+ const char *right = s;
+ int rv = 0;
+ // what area to scan.
+ // n may be npos, but we treat that as a huge positive value
+ size_type byteCount = min(length(), n);
+
+ // loop until we find a difference, a '\0', or reach the end of area to
scan
+ if (isCaseSensitive == caseSensitive) {
+ while ((rv = *left - *right++) == 0) {
+ if (*left++ == '\0' || --byteCount == 0)
+ break;
+ }
+ } else {
+ while ((rv = tolower(*left) - tolower(*right++)) == 0) {
+ if (*left++ == '\0' || --byteCount == 0)
+ break;
+ }
+ }
+
+ // If we stopped scanning because we reached the end of buf()
+ // pretend we have a 0-terminator there to compare.
+ // NP: the loop already incremented "right" ready for this comparison
+ if (!byteCount && length() < n)
+ return '\0' - *right;
+
+ // If we found a difference within the scan area,
+ // or we found a '\0',
+ // or all n characters were identical (and none was \0).
+ return rv;
+}
+
bool
-SBuf::startsWith(const SBuf &S, SBufCaseSensitive isCaseSensitive) const
+SBuf::startsWith(const SBuf &S, const SBufCaseSensitive isCaseSensitive) const
{
debugs(24, 8, id << " startsWith " << S.id << ", caseSensitive: " <<
isCaseSensitive);
if (length() < S.length()) {
debugs(24, 8, "no, too short");
++stats.compareFast;
return false;
}
return (compare(S, isCaseSensitive, S.length()) == 0);
}
bool
SBuf::operator ==(const SBuf & S) const
{
debugs(24, 8, id << " == " << S.id);
if (length() != S.length()) {
debugs(24, 8, "no, different lengths");
++stats.compareFast;
return false; //shortcut: must be equal length
}
=== modified file 'src/SBuf.h'
--- src/SBuf.h 2014-04-06 07:08:04 +0000
+++ src/SBuf.h 2014-04-16 17:49:48 +0000
@@ -238,58 +238,71 @@
char at(size_type pos) const {checkAccessBounds(pos); return
operator[](pos);}
/** direct-access set a byte at a specified operation.
*
* \param pos the position to be overwritten
* \param toset the value to be written
* \throw OutOfBoundsException when pos is of bounds
* \note bounds is 0 <= pos < length(); caller must pay attention to
signedness
* \note performs a copy-on-write if needed.
*/
void setAt(size_type pos, char toset);
/** compare to other SBuf, str(case)cmp-style
*
* \param isCaseSensitive one of caseSensitive or caseInsensitive
* \param n compare up to this many bytes. if npos (default), compare
whole SBufs
* \retval >0 argument of the call is greater than called SBuf
* \retval <0 argument of the call is smaller than called SBuf
* \retval 0 argument of the call has the same contents of called SBuf
*/
- int compare(const SBuf &S, SBufCaseSensitive isCaseSensitive, size_type n
= npos) const;
+ int compare(const SBuf &S, const SBufCaseSensitive isCaseSensitive, const
size_type n = npos) const;
- /// shorthand version for compare
- inline int cmp(const SBuf &S, size_type n = npos) const {
+ /// shorthand version for compare()
+ inline int cmp(const SBuf &S, const size_type n = npos) const {
return compare(S,caseSensitive,n);
}
- /// shorthand version for case-insensitive comparison
- inline int caseCmp(const SBuf &S, size_type n = npos) const {
+ /// shorthand version for case-insensitive compare()
+ inline int caseCmp(const SBuf &S, const size_type n = npos) const {
+ return compare(S,caseInsensitive,n);
+ }
+
+ /// Comparison with a C-string.
+ int compare(const char *s, const SBufCaseSensitive isCaseSensitive, const
size_type n = npos) const;
+
+ /// Shorthand version for C-string compare().
+ inline int cmp(const char *S, const size_type n = npos) const {
+ return compare(S,caseSensitive,n);
+ }
+
+ /// Shorthand version for case-insensitive C-string compare().
+ inline int caseCmp(const char *S, const size_type n = npos) const {
return compare(S,caseInsensitive,n);
}
/** check whether the entire supplied argument is a prefix of the SBuf.
* \param S the prefix to match against
* \param isCaseSensitive one of caseSensitive or caseInsensitive
* \retval true argument is a prefix of the SBuf
*/
- bool startsWith(const SBuf &S, SBufCaseSensitive isCaseSensitive =
caseSensitive) const;
+ bool startsWith(const SBuf &S, const SBufCaseSensitive isCaseSensitive =
caseSensitive) const;
bool operator ==(const SBuf & S) const;
bool operator !=(const SBuf & S) const;
bool operator <(const SBuf &S) const {return (cmp(S) < 0);}
bool operator >(const SBuf &S) const {return (cmp(S) > 0);}
bool operator <=(const SBuf &S) const {return (cmp(S) <= 0);}
bool operator >=(const SBuf &S) const {return (cmp(S) >= 0);}
/** Consume bytes at the head of the SBuf
*
* Consume N chars at SBuf head, or to SBuf's end,
* whichever is shorter. If more bytes are consumed than available,
* the SBuf is emptied
* \param n how many bytes to remove; could be zero.
* npos (or no argument) means 'to the end of SBuf'
* \return a new SBuf containing the consumed bytes.
*/
SBuf consume(size_type n = npos);
/// gets global statistic informations
